[apparmor] [PATCH 2/7] Add base function to query generic label data under a, given key

Seth Arnold seth.arnold at canonical.com
Wed Feb 15 02:56:26 UTC 2017


On Fri, Feb 10, 2017 at 12:47:31PM -0800, John Johansen wrote:
> +typedef struct {
> +	size_t size;			/* length of s */
> +	const char *entry;		/* not necessarily NULL-terminated */
> +} aa_label_data_ent;
> +
> +typedef struct {
> +	char *data;			/* free data */
> +	size_t n;			/* number of ents */
> +	aa_label_data_ent *ents;	/* free vec of entries */
> +} aa_label_data_info;

Note that these use size_t, which can vary between 32 bit and 64 bit
platforms.

> +int aa_query_label_data(const char *label, const char *key, aa_label_data_info *out)
> +{
> +	autofclose FILE *file = NULL;
> +	const char *p;
> +	uint32_t bytes;
> +	uint32_t tmp;
> +	int fd;
> +
> +	if (!out)
> +		return 0;
> +
> +	memset(out, 0, sizeof(*out));
> +
> +	fd = aa_query_label_data_open(label, key);
> +	if (fd == -1)
> +		return -1;
> +
> +	file = fdopen(fd, "r+");
> +	if (!file)
> +		return -1;
> +
> +	if (fread(&tmp, sizeof(tmp), 1, file) != 1) {
> +		errno = EPROTO;
> +		goto done;
> +	}
> +
> +	bytes = le32toh(tmp);
> +	out->data = malloc(bytes);
> +	if (!out->data) {
> +		errno = ENOMEM;
> +		goto done;
> +	}

But here, only 32 bits is used.

> +
> +	if (fread(out->data, sizeof(char), bytes, file) != bytes) {
> +		errno = EPROTO;
> +		goto done;
> +	}
> +
> +	p = out->data;
> +	memcpy(&tmp, p, sizeof(tmp));
> +	out->n = le32toh(tmp);
> +	p += sizeof(tmp);

And here, only 32 bits is used.

Is this a problem? I suspect it's fine but wanted to point it out anyway.

> +
> +	out->ents = malloc(out->n * sizeof(*out->ents));

If out->n is large enough, this multiplication might overflow. calloc()
would suffice but would be slightly wasteful.

> +	if (!out->data) {
> +		errno = ENOMEM;
> +		goto done;
> +	}
> +
> +	for (int i = 0; i < out->n; i++) {
> +		memcpy(&tmp, p, sizeof(tmp));
> +		out->ents[i].size = le32toh(tmp);
> +		p += sizeof(tmp);
> +		out->ents[i].entry = p;
> +		p += out->ents[i].size;
> +	}
> +
> +done:
> +	if (errno)
> +		aa_clear_label_data(out);
> +
> +	return errno ? -1 : 0;
> +}

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/20170214/cbf6161c/attachment.pgp>


More information about the AppArmor mailing list