[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