Re: [AppArmor 37/41] AppArmor: Main Part

Alan Cox
2007-04-12T10:34:54Z

> + * aa_taskattr_access
> + * @name: name of the file to check
> + *
> + * Check if name matches /proc/self/attr/current, with self resolved
> + * to the current pid. This file is the usermode iterface for
> + * changing one's hat.
> + */
> +static inline int aa_taskattr_access(const char *name)
> +{
> + unsigned long pid;
> + char *end;
> +
> + if (strncmp(name, "/proc/", 6) != 0)
> + return 0;

The proc file system may not be mounted at /proc. There are environments
where this is done for good reason (eg not wanting the /proc info exposed
to a low trust environment). Another is when FUSE is providing an
arbitrated proc either by merging across clusters or by removing stuff.

> +static int aa_file_denied(struct aa_profile *profile, const char *name,
> + int mask)
> +{
> + int perms;
> +
> + /* Always allow write access to /proc/self/attr/current. */
> + if (mask == MAY_WRITE && aa_taskattr_access(name))
> + return 0;

Why can't this be done in the profile itself to avoid kernel special case
uglies and inflexibility ?

> + if (PTR_ERR(sa->name) == -ENOENT && (check & AA_CHECK_FD))
> + denied_mask = 0;

Now there is an interesting question. Is PTR_ERR() safe for kernel
pointers on all platforms or just for user ones ?