[apparmor] [789409 at bugs.launchpad.net: [Bug 789409] [NEW] /proc/[PID]/attr/current overwrite Null pointer dereference]

[John Johansen]

On 05/27/2011 08:03 PM, Kees Cook wrote:
> Hi,
> 
> In audit.c:
> /**
>  * aa_audit - Log a profile based audit event to the audit subsystem
>  * @type: audit type for the message
>  * @profile: profile to check against (NOT NULL)
>  * @gfp: allocation flags to use
>  * @sa: audit event (NOT NULL)
>  * @cb: optional callback fn for type specific fields (MAYBE NULL)
>  *
>  * Handle default message switching based off of audit mode flags
>  *
>  * Returns: error on failure
>  */
> int aa_audit(int type, struct aa_profile *profile, gfp_t gfp,
>              struct common_audit_data *sa,
>              void (*cb) (struct audit_buffer *, void *))
> {
>         BUG_ON(!profile);
> ...
> 
> profile must not be NULL. But in apparmor_setprocattr() in lsm.c:
> 
>                         return aa_audit(AUDIT_APPARMOR_DENIED, NULL, GFP_KERNEL,
>                                         &sa, NULL);
> 
> Should arg 2 be __aa_current_profile() instead of NULL? Will that return a
> sensible profile struct when unconfined? *I think* it looks like there is a
> per-namespace "unconfined" profile that every process should have attached
> when not under some other profile.
> 
Yep Kees your right.

Every process belongs to a namespace now and every namespace has at least a
default unconfined profile.  This seems to be a vestige of when we used the
NULL pointer to indicate unconfined.

Patch attached with full credit to kees as he nailed it.

-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0001-AppArmor-Fix-oops-in-apparmor_setprocattr-when-inval.patch
Type: text/x-patch
Size: 1351 bytes
Desc: not available
URL: <https://lists.ubuntu.com/archives/apparmor/attachments/20110527/95a00882/attachment.bin>