Messages in this thread | | | Date | Thu, 25 Sep 2008 11:59:54 -0500 | From | "Serge E. Hallyn" <> | Subject | Re: [TOMOYO #9 (2.6.27-rc7-mm1) 1/6] LSM adapter functions. |
| |
Quoting Kentaro Takeda (takedakn@nttdata.co.jp): > 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>
So IMO there is some major badness here in the form of copying all of those functions out of fs/namei.c. I think we need to discuss case-by-case whether using the functions is appropriate (and hence they should be made non-static in fs/namei.c), or whether the intended goal should be met some other way. For instance:
> --- > security/tomoyo/tomoyo.c | 341 +++++++++++++++++++++++++++++++++++++++++++++++ > security/tomoyo/tomoyo.h | 97 +++++++++++++ > 2 files changed, 438 insertions(+) >
...
> +static int tmy_path_mknod(struct path *parent, struct dentry *dentry, int mode, > + unsigned int dev) > +{ > + struct vfsmount *mnt = parent->mnt; > + struct inode *dir = parent->dentry->d_inode; > + int error = 0; > + > + switch (mode & S_IFMT) { > + case S_IFREG: > + case 0: > + error = may_create(dir, dentry);
Isn't may_create already done at the top of vfs_mknod() and vfs_create()?
> + if (error) > + return error; > + if (!dir->i_op || !dir->i_op->create) > + return -EACCES; /* shouldn't it be ENOSYS? */ > + break; > + case S_IFCHR: > + case S_IFBLK: > + case S_IFIFO: > + case S_IFSOCK: > + error = may_create(dir, dentry);
likewise...
> + if (error) > + return error; > + if ((S_ISCHR(mode) || S_ISBLK(mode)) && !capable(CAP_MKNOD)) > + return -EPERM;
likewise...
> + if (!dir->i_op || !dir->i_op->mknod) > + return -EPERM; > + error = devcgroup_inode_mknod(mode, dev);
likewise... (except in the case of fifo/sock, devcgroup should not be consulted as I'm not sure it'll handle that properly - have you tested tis with the device cgroup enabled?)
> + if (error) > + return error; > + break; > + default: > + return 0; > + } > + switch (mode & S_IFMT) { > + case S_IFREG: > + case 0: > + error = tmy_check_1path_perm(TMY_TYPE_CREATE_ACL, dentry, mnt); > + break; > + case S_IFCHR: > + error = tmy_check_1path_perm(TMY_TYPE_MKCHAR_ACL, dentry, mnt); > + break; > + case S_IFBLK: > + error = tmy_check_1path_perm(TMY_TYPE_MKBLOCK_ACL, dentry, mnt); > + break; > + case S_IFIFO: > + error = tmy_check_1path_perm(TMY_TYPE_MKFIFO_ACL, dentry, mnt); > + break; > + case S_IFSOCK: > + error = tmy_check_1path_perm(TMY_TYPE_MKSOCK_ACL, dentry, mnt); > + break; > + } > + return error; > +}
-serge
| |