linux-kernel - Re: [PATCH v2 5/5] ramfs: Initialize security of in-memory inodes

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <379f6a45a3102e8cd1ed992c1895ac9f69ae0595.camel@huaweicloud.com>
Date:   Wed, 15 Nov 2023 09:01:52 +0100
From:   Roberto Sassu <roberto.sassu@...weicloud.com>
To:     casey@...aufler-ca.com, paul@...l-moore.com, jmorris@...ei.org,
        serge@...lyn.com, Andrew Morton <akpm@...ux-foundation.org>
Cc:     linux-kernel@...r.kernel.org,
        linux-security-module@...r.kernel.org,
        Roberto Sassu <roberto.sassu@...wei.com>
Subject: Re: [PATCH v2 5/5] ramfs: Initialize security of in-memory inodes

On Mon, 2023-07-24 at 17:13 +0200, Roberto Sassu wrote:
> From: Roberto Sassu <roberto.sassu@...wei.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@...wei.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);
>  }