| Message ID | 20230724151341.538889-6-roberto.sassu@huaweicloud.com (mailing list archive) |
|---|---|
| State | Handled Elsewhere |
| Headers | show |
| Series | Smack transmute fixes | expand |
On Mon, 2023-07-24 at 17:13 +0200, Roberto Sassu wrote: > From: Roberto Sassu <roberto.sassu@huawei.com> > > Add a call security_inode_init_security() after ramfs_get_inode(), to let > LSMs initialize the inode security field. Skip ramfs_fill_super(), as the > initialization is done through the sb_set_mnt_opts hook. > > Calling security_inode_init_security() call inside ramfs_get_inode() is > not possible since, for CONFIG_SHMEM=n, tmpfs also calls the former after > the latter. > > Pass NULL as initxattrs() callback to security_inode_init_security(), since > the purpose of the call is only to initialize the in-memory inodes. > > Signed-off-by: Roberto Sassu <roberto.sassu@huawei.com> + Andrew Hi Andrew I'm proposing an extension to initialize the inode security field at inode creation time for filesystems that don't support xattrs (ramfs in this case). The LSM infrastructure already supports setting the inode security field, but only at run-time, with the inode_setsecurity hook. I developed this to do some testing on the Smack LSM, and I thought it could be useful anyway. Casey would need your acked-by, to carry this patch in his repository. I'm not completely sure if you are the maintainer, but in the past you accepted a patch for ramfs. If you have time and you could have a look, that would be great! Thanks Roberto > --- > fs/ramfs/inode.c | 27 +++++++++++++++++++++++++++ > 1 file changed, 27 insertions(+) > > diff --git a/fs/ramfs/inode.c b/fs/ramfs/inode.c > index fef477c7810..ac90ebd9dbd 100644 > --- a/fs/ramfs/inode.c > +++ b/fs/ramfs/inode.c > @@ -102,6 +102,14 @@ ramfs_mknod(struct mnt_idmap *idmap, struct inode *dir, > int error = -ENOSPC; > > if (inode) { > + error = security_inode_init_security(inode, dir, > + &dentry->d_name, NULL, > + NULL); > + if (error) { > + iput(inode); > + return error; > + } > + > d_instantiate(dentry, inode); > dget(dentry); /* Extra count - pin the dentry in core */ > error = 0; > @@ -134,6 +142,15 @@ static int ramfs_symlink(struct mnt_idmap *idmap, struct inode *dir, > inode = ramfs_get_inode(dir->i_sb, dir, S_IFLNK|S_IRWXUGO, 0); > if (inode) { > int l = strlen(symname)+1; > + > + error = security_inode_init_security(inode, dir, > + &dentry->d_name, NULL, > + NULL); > + if (error) { > + iput(inode); > + return error; > + } > + > error = page_symlink(inode, symname, l); > if (!error) { > d_instantiate(dentry, inode); > @@ -149,10 +166,20 @@ static int ramfs_tmpfile(struct mnt_idmap *idmap, > struct inode *dir, struct file *file, umode_t mode) > { > struct inode *inode; > + int error; > > inode = ramfs_get_inode(dir->i_sb, dir, mode, 0); > if (!inode) > return -ENOSPC; > + > + error = security_inode_init_security(inode, dir, > + &file_dentry(file)->d_name, NULL, > + NULL); > + if (error) { > + iput(inode); > + return error; > + } > + > d_tmpfile(file, inode); > return finish_open_simple(file, 0); > }
On Wed, 15 Nov 2023 09:01:52 +0100 Roberto Sassu <roberto.sassu@huaweicloud.com> wrote: > On Mon, 2023-07-24 at 17:13 +0200, Roberto Sassu wrote: > > From: Roberto Sassu <roberto.sassu@huawei.com> > > > > Add a call security_inode_init_security() after ramfs_get_inode(), to let > > LSMs initialize the inode security field. Skip ramfs_fill_super(), as the > > initialization is done through the sb_set_mnt_opts hook. > > > > Calling security_inode_init_security() call inside ramfs_get_inode() is > > not possible since, for CONFIG_SHMEM=n, tmpfs also calls the former after > > the latter. > > > > Pass NULL as initxattrs() callback to security_inode_init_security(), since > > the purpose of the call is only to initialize the in-memory inodes. > > > > Signed-off-by: Roberto Sassu <roberto.sassu@huawei.com> > > + Andrew > > Hi Andrew > > I'm proposing an extension to initialize the inode security field at > inode creation time for filesystems that don't support xattrs (ramfs in > this case). > > The LSM infrastructure already supports setting the inode security > field, but only at run-time, with the inode_setsecurity hook. > > I developed this to do some testing on the Smack LSM, and I thought it > could be useful anyway. > > Casey would need your acked-by, to carry this patch in his repository. > I'm not completely sure if you are the maintainer, but in the past you > accepted a patch for ramfs. > > If you have time and you could have a look, that would be great! Patch looks OK to me. Please cc Hugh and myself on a resend. One little thing: > > +++ b/fs/ramfs/inode.c > > @@ -102,6 +102,14 @@ ramfs_mknod(struct mnt_idmap *idmap, struct inode *dir, > > int error = -ENOSPC; > > > > if (inode) { > > + error = security_inode_init_security(inode, dir, > > + &dentry->d_name, NULL, > > + NULL); > > + if (error) { > > + iput(inode); > > + return error; A `break' here would be better. To avoid having multiple return points, which are often a maintenance hassle. Same treatment at the other sites.
diff --git a/fs/ramfs/inode.c b/fs/ramfs/inode.c index fef477c7810..ac90ebd9dbd 100644 --- a/fs/ramfs/inode.c +++ b/fs/ramfs/inode.c @@ -102,6 +102,14 @@ ramfs_mknod(struct mnt_idmap *idmap, struct inode *dir, int error = -ENOSPC; if (inode) { + error = security_inode_init_security(inode, dir, + &dentry->d_name, NULL, + NULL); + if (error) { + iput(inode); + return error; + } + d_instantiate(dentry, inode); dget(dentry); /* Extra count - pin the dentry in core */ error = 0; @@ -134,6 +142,15 @@ static int ramfs_symlink(struct mnt_idmap *idmap, struct inode *dir, inode = ramfs_get_inode(dir->i_sb, dir, S_IFLNK|S_IRWXUGO, 0); if (inode) { int l = strlen(symname)+1; + + error = security_inode_init_security(inode, dir, + &dentry->d_name, NULL, + NULL); + if (error) { + iput(inode); + return error; + } + error = page_symlink(inode, symname, l); if (!error) { d_instantiate(dentry, inode); @@ -149,10 +166,20 @@ static int ramfs_tmpfile(struct mnt_idmap *idmap, struct inode *dir, struct file *file, umode_t mode) { struct inode *inode; + int error; inode = ramfs_get_inode(dir->i_sb, dir, mode, 0); if (!inode) return -ENOSPC; + + error = security_inode_init_security(inode, dir, + &file_dentry(file)->d_name, NULL, + NULL); + if (error) { + iput(inode); + return error; + } + d_tmpfile(file, inode); return finish_open_simple(file, 0); }