diff mbox series

[v3,5/5] ramfs: Initialize security of in-memory inodes

Message ID 20231116090125.187209-6-roberto.sassu@huaweicloud.com (mailing list archive)
State Handled Elsewhere
Headers show
Series Smack transmute fixes | expand

Commit Message

Roberto Sassu Nov. 16, 2023, 9:01 a.m. UTC
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.

Cc: Hugh Dickins <hughd@google.com>
Cc: Andrew Morton <akpm@linux-foundation.org>
Signed-off-by: Roberto Sassu <roberto.sassu@huawei.com>
---
 fs/ramfs/inode.c | 32 +++++++++++++++++++++++++++++++-
 1 file changed, 31 insertions(+), 1 deletion(-)

Comments

Roberto Sassu Jan. 12, 2024, 8:17 a.m. UTC | #1
On Thu, 2023-11-16 at 10:01 +0100, 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.

Hugh, Andrew, is the patch fine for you? Casey would make a PR for the
patch set.

Thanks

Roberto

> Cc: Hugh Dickins <hughd@google.com>
> Cc: Andrew Morton <akpm@linux-foundation.org>
> Signed-off-by: Roberto Sassu <roberto.sassu@huawei.com>
> ---
>  fs/ramfs/inode.c | 32 +++++++++++++++++++++++++++++++-
>  1 file changed, 31 insertions(+), 1 deletion(-)
> 
> diff --git a/fs/ramfs/inode.c b/fs/ramfs/inode.c
> index 4ac05a9e25bc..8006faaaf0ec 100644
> --- a/fs/ramfs/inode.c
> +++ b/fs/ramfs/inode.c
> @@ -102,11 +102,20 @@ 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);
> +			goto out;
> +		}
> +
>  		d_instantiate(dentry, inode);
>  		dget(dentry);	/* Extra count - pin the dentry in core */
>  		error = 0;
>  		inode_set_mtime_to_ts(dir, inode_set_ctime_current(dir));
>  	}
> +out:
>  	return error;
>  }
>  
> @@ -134,6 +143,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);
> +			goto out;
> +		}
> +
>  		error = page_symlink(inode, symname, l);
>  		if (!error) {
>  			d_instantiate(dentry, inode);
> @@ -143,6 +161,7 @@ static int ramfs_symlink(struct mnt_idmap *idmap, struct inode *dir,
>  		} else
>  			iput(inode);
>  	}
> +out:
>  	return error;
>  }
>  
> @@ -150,12 +169,23 @@ 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);
> +		goto out;
> +	}
> +
>  	d_tmpfile(file, inode);
> -	return finish_open_simple(file, 0);
> +out:
> +	return finish_open_simple(file, error);
>  }
>  
>  static const struct inode_operations ramfs_dir_inode_operations = {
Roberto Sassu Jan. 16, 2024, 8:39 a.m. UTC | #2
On Fri, 2024-01-12 at 09:17 +0100, Roberto Sassu wrote:
> On Thu, 2023-11-16 at 10:01 +0100, 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.
> 
> Hugh, Andrew, is the patch fine for you? Casey would make a PR for the
> patch set.

(I guess putting the people of interest in To instead of CC could
help...)

Friendly ping... it would be awesome if you could Ack this patch so
that Casey can still ask Linus to pull.

Thanks

Roberto

> Thanks
> 
> Roberto
> 
> > Cc: Hugh Dickins <hughd@google.com>
> > Cc: Andrew Morton <akpm@linux-foundation.org>
> > Signed-off-by: Roberto Sassu <roberto.sassu@huawei.com>
> > ---
> >  fs/ramfs/inode.c | 32 +++++++++++++++++++++++++++++++-
> >  1 file changed, 31 insertions(+), 1 deletion(-)
> > 
> > diff --git a/fs/ramfs/inode.c b/fs/ramfs/inode.c
> > index 4ac05a9e25bc..8006faaaf0ec 100644
> > --- a/fs/ramfs/inode.c
> > +++ b/fs/ramfs/inode.c
> > @@ -102,11 +102,20 @@ 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);
> > +			goto out;
> > +		}
> > +
> >  		d_instantiate(dentry, inode);
> >  		dget(dentry);	/* Extra count - pin the dentry in core */
> >  		error = 0;
> >  		inode_set_mtime_to_ts(dir, inode_set_ctime_current(dir));
> >  	}
> > +out:
> >  	return error;
> >  }
> >  
> > @@ -134,6 +143,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);
> > +			goto out;
> > +		}
> > +
> >  		error = page_symlink(inode, symname, l);
> >  		if (!error) {
> >  			d_instantiate(dentry, inode);
> > @@ -143,6 +161,7 @@ static int ramfs_symlink(struct mnt_idmap *idmap, struct inode *dir,
> >  		} else
> >  			iput(inode);
> >  	}
> > +out:
> >  	return error;
> >  }
> >  
> > @@ -150,12 +169,23 @@ 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);
> > +		goto out;
> > +	}
> > +
> >  	d_tmpfile(file, inode);
> > -	return finish_open_simple(file, 0);
> > +out:
> > +	return finish_open_simple(file, error);
> >  }
> >  
> >  static const struct inode_operations ramfs_dir_inode_operations = {
>
Andrew Morton Jan. 26, 2024, 1:08 a.m. UTC | #3
On Thu, 16 Nov 2023 10:01:25 +0100 Roberto Sassu <roberto.sassu@huaweicloud.com> 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.
> 

fwiw,

Acked-by: Andrew Morton <akpm@linux-foundation.org>

Please include this in the relevant security tree.

> diff --git a/fs/ramfs/inode.c b/fs/ramfs/inode.c
> index 4ac05a9e25bc..8006faaaf0ec 100644
> --- a/fs/ramfs/inode.c
> +++ b/fs/ramfs/inode.c
> @@ -102,11 +102,20 @@ 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);
> +			goto out;
> +		}
> +
>  		d_instantiate(dentry, inode);
>  		dget(dentry);	/* Extra count - pin the dentry in core */
>  		error = 0;
>  		inode_set_mtime_to_ts(dir, inode_set_ctime_current(dir));
>  	}
> +out:
>  	return error;
>  }
>  
> @@ -134,6 +143,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);
> +			goto out;
> +		}
> +
>  		error = page_symlink(inode, symname, l);
>  		if (!error) {
>  			d_instantiate(dentry, inode);
> @@ -143,6 +161,7 @@ static int ramfs_symlink(struct mnt_idmap *idmap, struct inode *dir,
>  		} else
>  			iput(inode);
>  	}
> +out:
>  	return error;
>  }
>  
> @@ -150,12 +169,23 @@ 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);
> +		goto out;
> +	}
> +
>  	d_tmpfile(file, inode);
> -	return finish_open_simple(file, 0);
> +out:
> +	return finish_open_simple(file, error);
>  }
>  
>  static const struct inode_operations ramfs_dir_inode_operations = {
> -- 
> 2.34.1
Roberto Sassu Jan. 26, 2024, 7:53 a.m. UTC | #4
On Thu, 2024-01-25 at 17:08 -0800, Andrew Morton wrote:
> On Thu, 16 Nov 2023 10:01:25 +0100 Roberto Sassu <roberto.sassu@huaweicloud.com> 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.
> > 
> 
> fwiw,
> 
> Acked-by: Andrew Morton <akpm@linux-foundation.org>
> 
> Please include this in the relevant security tree.

Thanks a lot!

Roberto

> > diff --git a/fs/ramfs/inode.c b/fs/ramfs/inode.c
> > index 4ac05a9e25bc..8006faaaf0ec 100644
> > --- a/fs/ramfs/inode.c
> > +++ b/fs/ramfs/inode.c
> > @@ -102,11 +102,20 @@ 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);
> > +			goto out;
> > +		}
> > +
> >  		d_instantiate(dentry, inode);
> >  		dget(dentry);	/* Extra count - pin the dentry in core */
> >  		error = 0;
> >  		inode_set_mtime_to_ts(dir, inode_set_ctime_current(dir));
> >  	}
> > +out:
> >  	return error;
> >  }
> >  
> > @@ -134,6 +143,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);
> > +			goto out;
> > +		}
> > +
> >  		error = page_symlink(inode, symname, l);
> >  		if (!error) {
> >  			d_instantiate(dentry, inode);
> > @@ -143,6 +161,7 @@ static int ramfs_symlink(struct mnt_idmap *idmap, struct inode *dir,
> >  		} else
> >  			iput(inode);
> >  	}
> > +out:
> >  	return error;
> >  }
> >  
> > @@ -150,12 +169,23 @@ 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);
> > +		goto out;
> > +	}
> > +
> >  	d_tmpfile(file, inode);
> > -	return finish_open_simple(file, 0);
> > +out:
> > +	return finish_open_simple(file, error);
> >  }
> >  
> >  static const struct inode_operations ramfs_dir_inode_operations = {
> > -- 
> > 2.34.1
Casey Schaufler Jan. 26, 2024, 4:27 p.m. UTC | #5
On 1/25/2024 11:53 PM, Roberto Sassu wrote:
> On Thu, 2024-01-25 at 17:08 -0800, Andrew Morton wrote:
>> On Thu, 16 Nov 2023 10:01:25 +0100 Roberto Sassu <roberto.sassu@huaweicloud.com> 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.
>>>
>> fwiw,
>>
>> Acked-by: Andrew Morton <akpm@linux-foundation.org>
>>
>> Please include this in the relevant security tree.

I will take this in the Smack tree. Thank you.

> Thanks a lot!
>
> Roberto
>
>>> diff --git a/fs/ramfs/inode.c b/fs/ramfs/inode.c
>>> index 4ac05a9e25bc..8006faaaf0ec 100644
>>> --- a/fs/ramfs/inode.c
>>> +++ b/fs/ramfs/inode.c
>>> @@ -102,11 +102,20 @@ 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);
>>> +			goto out;
>>> +		}
>>> +
>>>  		d_instantiate(dentry, inode);
>>>  		dget(dentry);	/* Extra count - pin the dentry in core */
>>>  		error = 0;
>>>  		inode_set_mtime_to_ts(dir, inode_set_ctime_current(dir));
>>>  	}
>>> +out:
>>>  	return error;
>>>  }
>>>  
>>> @@ -134,6 +143,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);
>>> +			goto out;
>>> +		}
>>> +
>>>  		error = page_symlink(inode, symname, l);
>>>  		if (!error) {
>>>  			d_instantiate(dentry, inode);
>>> @@ -143,6 +161,7 @@ static int ramfs_symlink(struct mnt_idmap *idmap, struct inode *dir,
>>>  		} else
>>>  			iput(inode);
>>>  	}
>>> +out:
>>>  	return error;
>>>  }
>>>  
>>> @@ -150,12 +169,23 @@ 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);
>>> +		goto out;
>>> +	}
>>> +
>>>  	d_tmpfile(file, inode);
>>> -	return finish_open_simple(file, 0);
>>> +out:
>>> +	return finish_open_simple(file, error);
>>>  }
>>>  
>>>  static const struct inode_operations ramfs_dir_inode_operations = {
>>> -- 
>>> 2.34.1
>
diff mbox series

Patch

diff --git a/fs/ramfs/inode.c b/fs/ramfs/inode.c
index 4ac05a9e25bc..8006faaaf0ec 100644
--- a/fs/ramfs/inode.c
+++ b/fs/ramfs/inode.c
@@ -102,11 +102,20 @@  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);
+			goto out;
+		}
+
 		d_instantiate(dentry, inode);
 		dget(dentry);	/* Extra count - pin the dentry in core */
 		error = 0;
 		inode_set_mtime_to_ts(dir, inode_set_ctime_current(dir));
 	}
+out:
 	return error;
 }
 
@@ -134,6 +143,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);
+			goto out;
+		}
+
 		error = page_symlink(inode, symname, l);
 		if (!error) {
 			d_instantiate(dentry, inode);
@@ -143,6 +161,7 @@  static int ramfs_symlink(struct mnt_idmap *idmap, struct inode *dir,
 		} else
 			iput(inode);
 	}
+out:
 	return error;
 }
 
@@ -150,12 +169,23 @@  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);
+		goto out;
+	}
+
 	d_tmpfile(file, inode);
-	return finish_open_simple(file, 0);
+out:
+	return finish_open_simple(file, error);
 }
 
 static const struct inode_operations ramfs_dir_inode_operations = {