Messages in this thread | | | Date | Thu, 9 Oct 2008 15:10:06 +0900 | From | KAMEZAWA Hiroyuki <> | Subject | Re: [TOMOYO #10 (linux-next) 3/8] LSM adapter functions. |
| |
Hi,
some nitpicks.
On Thu, 09 Oct 2008 13:28:17 +0900 Kentaro Takeda <takedakn@nttdata.co.jp> wrote:
> Signed-off-by: Kentaro Takeda <takedakn@nttdata.co.jp> > Signed-off-by: Tetsuo Handa <penguin-kernel@I-love.SAKURA.ne.jp> > Signed-off-by: Toshiharu Harada <haradats@nttdata.co.jp> > --- > security/tomoyo/tomoyo.c | 319 +++++++++++++++++++++++++++++++++++++++++++++++ > security/tomoyo/tomoyo.h | 106 +++++++++++++++ > 2 files changed, 425 insertions(+) > > --- /dev/null > +++ linux-next/security/tomoyo/tomoyo.c > @@ -0,0 +1,319 @@ > +/* > + * security/tomoyo/tomoyo.c > + * > + * LSM hooks for TOMOYO Linux. > + * > + * Copyright (C) 2005-2008 NTT DATA CORPORATION > + * > + * Version: 2.2.0-pre 2008/10/01 > + * > + */
What is this Version for ? Is this exported to userland via some interface ?
> + > +#include <linux/security.h> > +#include "common.h" > +#include "tomoyo.h" > +#include "realpath.h" > +#include <linux/audit.h> > +#include <linux/device_cgroup.h> > + > +static int tmy_cred_prepare(struct cred *new, const struct cred *old, gfp_t gfp) > +{ > + new->security = old->security; > + return 0; > +} > + > +static int tmy_bprm_set_creds(struct linux_binprm *bprm) > +{ > + if (bprm->cred_prepared) > + return 0; > + if (!sbin_init_started) > + tmy_load_policy(bprm->filename); > + bprm->cred->security = NULL; > + return 0; > +} > + > +static int tmy_bprm_check_security(struct linux_binprm *bprm) > +{ > + struct domain_info *domain = bprm->cred->security; > + /* > + * Execute permission is checked against pathname passed to do_execve() > + * using current domain. > + */ > + if (!domain) { > + struct domain_info *next_domain = NULL; > + int retval = tmy_find_next_domain(bprm, &next_domain); > + if (!retval) > + bprm->cred->security = next_domain; > + return retval; > + } > + /* > + * Read permission is checked against interpreters using next domain. > + */ > + return tmy_check_open_permission(domain, bprm->file->f_path.dentry, > + bprm->file->f_path.mnt, 1); > +} > + > +static int tmy_sysctl(struct ctl_table *table, int op) > +{ > + int error; > + char *name; > + if ((op & 6) == 0) > + return 0;
It seems that you need a blanc line between variable declaration and start of code, like this.
== char *name; << if ((op & 6) == 0) return 0; ==
BTW, is this "6" need to be "6" rather than some readable macro ?
<snip> > +static int tmy_dentry_open(struct file *f, const struct cred *cred) > +{ > + int flags = f->f_flags; > + if ((flags + 1) & O_ACCMODE) > + flags++; > + flags |= f->f_flags & (O_APPEND | O_TRUNC); > + /* Don't check read permission here if called from do_execve(). */ > + if (current->in_execve) > + return 0; > + return tmy_check_open_permission(tmy_domain(), f->f_path.dentry, > + f->f_path.mnt, flags); > +} > +
This tmy_check_open_permission() is defiend in 7/8. Because people uses "bisect" in these days, I think it's better to avoid this kind of reversed dependency of patch stack.
Thanks, -Kame
| |