[apparmor] [PATCH 4/7] Add support for dconf confinement

John Johansen john.johansen at canonical.com
Thu Feb 16 04:52:22 UTC 2017


On 02/15/2017 08:46 PM, Seth Arnold wrote:
> This patch was mostly good with a few questions:
> 
> Also, I noticed all the copyright years need to be updated.
> 
> On Fri, Feb 10, 2017 at 12:51:49PM -0800, John Johansen wrote:
>> +	info->rpaths = malloc(info->rn * sizeof(*info->rpaths));
>> +	info->rwpaths = malloc(info->rwn * sizeof(*info->rwpaths));
>> +	info->arpaths = malloc(info->arn * sizeof(*info->arpaths));
>> +	info->arwpaths = malloc(info->arwn * sizeof(*info->arwpaths));
> 
> I'd rather see these as calloc() or similar overflow-gaurding methods.
> 
>> +std::ostream &dconf_rule::dump(std::ostream &os)
>> +{
>> +	if (audit)
>> +		os << "audit ";
>> +
>> +	os << "dconf (";
>> +
>> +	switch (mode & AA_DCONF_READWRITE) {
>> +	case AA_DCONF_READ:
>> +		os << "read";
>> +		break;
>> +	case AA_DCONF_WRITE:
>> +		os << " write";
>> +		break;
>> +	}
>> +	os << ") " << path << ",\n";
>> +
>> +	return os;
>> +}
> 
> I think it's unspecified what will happen if both read and write are
> selected here; I'm pretty sure the grammar allows both to be enabled
> at once.
> 
The grammar has a semantic check in the yacc portion requiring either
r or rw  but not w

> 
>> +class DataEnt {
>> +public:
>> +	virtual ~DataEnt() = 0;
>> +	virtual void serialize(std::ostringstream &buf) = 0;
>> +};
>> +/* just in case the base case destructor ever gets invoked */
>> +inline DataEnt::~DataEnt() { };
> 
> Is it kosher to set it to 0 above but then define it here?
> 
it won't break things but yeah should fix.




More information about the AppArmor mailing list