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

Seth Arnold seth.arnold at canonical.com
Thu Feb 16 04:46:21 UTC 2017


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.


> +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?

Thanks
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 473 bytes
Desc: not available
URL: <https://lists.ubuntu.com/archives/apparmor/attachments/20170215/72ced316/attachment.pgp>


More information about the AppArmor mailing list