Re: [Patch 0/12] AppArmor security module

[Tetsuo Handa]

Hello.

I browsed using lxr.



> static int aa_audit_caps(struct aa_profile *profile, struct aa_audit_caps *sa)
...snipped...
> 	ent = &get_cpu_var(audit_cache);
> 	if (sa->base.task == ent->task && cap_raised(ent->caps, sa->cap)) {

		put_cpu_var(audit_cache); ?

> 		if (PROFILE_COMPLAIN(profile))
> 			return 0;
> 		return sa->base.error;
> 	} else {
> 		ent->task = sa->base.task;
> 		cap_raise(ent->caps, sa->cap);
> 	}
> 	put_cpu_var(audit_cache);
...snipped...



Regarding unpack_*(), I'm not sure, but e seems to be no longer used after once
unpack_*() failed. If so, we can remove

> 	void *pos = e->pos;

and

>  fail:
>  	e->pos = pos;



Also, please add comments regarding

  memory allocated here is released by ...

  refcount obtained here is released by ...

  the caller of this function need to hold ... lock

as it is difficult for me to track memleak/refcounter/locking bugs.
For example, in function apparmor_dentry_open(), from

	fcxt->profile = aa_get_profile(profile);

to something like

	/* released by ... */
	fcxt->profile = aa_get_profile(profile);

(Oh, is it correct to get refcount even if aa_path_perm() failed?)



Regards.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/