Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.32 as permitted sender) client-ip=23.128.96.32;
Message-ID: <379f6a45a3102e8cd1ed992c1895ac9f69ae0595.camel@huaweicloud.com>
Subject: Re: [PATCH v2 5/5] ramfs: Initialize security of in-memory inodes
From:   Roberto Sassu <roberto.sassu@huaweicloud.com>
To:     casey@schaufler-ca.com, paul@paul-moore.com, jmorris@namei.org,
        serge@hallyn.com, Andrew Morton <akpm@linux-foundation.org>
Cc:     linux-kernel@vger.kernel.org,
        linux-security-module@vger.kernel.org,
        Roberto Sassu <roberto.sassu@huawei.com>
Date:   Wed, 15 Nov 2023 09:01:52 +0100
In-Reply-To: <20230724151341.538889-6-roberto.sassu@huaweicloud.com>
References: <20230724151341.538889-1-roberto.sassu@huaweicloud.com>
         <20230724151341.538889-6-roberto.sassu@huaweicloud.com>
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: 8BIT
User-Agent: Evolution 3.44.4-0ubuntu2 
MIME-Version: 1.0
Precedence: bulk

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);
>  }