[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-Id: <200911050549.nA55nQt1094958@www262.sakura.ne.jp>
Date: Thu, 05 Nov 2009 14:49:26 +0900
From: Tetsuo Handa <penguin-kernel@...ove.SAKURA.ne.jp>
To: john.johansen@...onical.com
Cc: linux-security-module@...r.kernel.org, linux-kernel@...r.kernel.org
Subject: Re: [Patch 0/12] AppArmor security module
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@...r.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Powered by blists - more mailing lists