2021-12-10 19:48:29

by Stefan Berger

[permalink] [raw]
Subject: [PATCH v6 12/17] securityfs: Extend securityfs with namespacing support

Extend 'securityfs' for support of IMA namespacing so that each
IMA (user) namespace can have its own front-end for showing the currently
active policy, the measurement list, number of violations and so on.

Drop the addition dentry reference and simplify cleanup to work without
the additional reference. This enables simple cleanup of dentries upon
umount.

Signed-off-by: Stefan Berger <[email protected]>
Signed-off-by: James Bottomley <[email protected]>
---
security/inode.c | 14 ++++----------
1 file changed, 4 insertions(+), 10 deletions(-)

diff --git a/security/inode.c b/security/inode.c
index fee01ff4d831..c77ae8ecc464 100644
--- a/security/inode.c
+++ b/security/inode.c
@@ -54,7 +54,7 @@ static int securityfs_fill_super(struct super_block *sb, struct fs_context *fc)

static int securityfs_get_tree(struct fs_context *fc)
{
- return get_tree_single(fc, securityfs_fill_super);
+ return get_tree_keyed(fc, securityfs_fill_super, fc->user_ns);
}

static const struct fs_context_operations securityfs_context_ops = {
@@ -72,6 +72,7 @@ static struct file_system_type fs_type = {
.name = "securityfs",
.init_fs_context = securityfs_init_fs_context,
.kill_sb = kill_litter_super,
+ .fs_flags = FS_USERNS_MOUNT,
};

/**
@@ -168,7 +169,6 @@ static struct dentry *securityfs_create_dentry(const char *name, umode_t mode,
inode->i_fop = fops;
}
d_instantiate(dentry, inode);
- dget(dentry);
inode_unlock(dir);
return dentry;

@@ -306,23 +306,17 @@ EXPORT_SYMBOL_GPL(securityfs_create_symlink);
void securityfs_remove(struct dentry *dentry)
{
struct user_namespace *ns;
- struct inode *dir;

if (!dentry || IS_ERR(dentry))
return;

ns = dentry->d_sb->s_user_ns;

- dir = d_inode(dentry->d_parent);
- inode_lock(dir);
if (simple_positive(dentry)) {
- if (d_is_dir(dentry))
- simple_rmdir(dir, dentry);
- else
- simple_unlink(dir, dentry);
+ d_delete(dentry);
dput(dentry);
}
- inode_unlock(dir);
+
if (ns == &init_user_ns)
simple_release_fs(&init_securityfs_mount,
&init_securityfs_mount_count);
--
2.31.1



2021-12-11 10:50:41

by Christian Brauner

[permalink] [raw]
Subject: Re: [PATCH v6 12/17] securityfs: Extend securityfs with namespacing support

On Fri, Dec 10, 2021 at 02:47:31PM -0500, Stefan Berger wrote:
> Extend 'securityfs' for support of IMA namespacing so that each
> IMA (user) namespace can have its own front-end for showing the currently
> active policy, the measurement list, number of violations and so on.
>
> Drop the addition dentry reference and simplify cleanup to work without
> the additional reference. This enables simple cleanup of dentries upon
> umount.
>
> Signed-off-by: Stefan Berger <[email protected]>
> Signed-off-by: James Bottomley <[email protected]>
> ---
> security/inode.c | 14 ++++----------
> 1 file changed, 4 insertions(+), 10 deletions(-)
>
> diff --git a/security/inode.c b/security/inode.c
> index fee01ff4d831..c77ae8ecc464 100644
> --- a/security/inode.c
> +++ b/security/inode.c
> @@ -54,7 +54,7 @@ static int securityfs_fill_super(struct super_block *sb, struct fs_context *fc)
>
> static int securityfs_get_tree(struct fs_context *fc)
> {
> - return get_tree_single(fc, securityfs_fill_super);
> + return get_tree_keyed(fc, securityfs_fill_super, fc->user_ns);
> }
>
> static const struct fs_context_operations securityfs_context_ops = {
> @@ -72,6 +72,7 @@ static struct file_system_type fs_type = {
> .name = "securityfs",
> .init_fs_context = securityfs_init_fs_context,
> .kill_sb = kill_litter_super,
> + .fs_flags = FS_USERNS_MOUNT,
> };
>
> /**
> @@ -168,7 +169,6 @@ static struct dentry *securityfs_create_dentry(const char *name, umode_t mode,
> inode->i_fop = fops;
> }
> d_instantiate(dentry, inode);
> - dget(dentry);
> inode_unlock(dir);
> return dentry;
>
> @@ -306,23 +306,17 @@ EXPORT_SYMBOL_GPL(securityfs_create_symlink);
> void securityfs_remove(struct dentry *dentry)
> {
> struct user_namespace *ns;
> - struct inode *dir;
>
> if (!dentry || IS_ERR(dentry))
> return;
>
> ns = dentry->d_sb->s_user_ns;
>
> - dir = d_inode(dentry->d_parent);
> - inode_lock(dir);
> if (simple_positive(dentry)) {
> - if (d_is_dir(dentry))
> - simple_rmdir(dir, dentry);
> - else
> - simple_unlink(dir, dentry);
> + d_delete(dentry);
> dput(dentry);

In
https://lore.kernel.org/lkml/20211210114934.tacjnwryihrsx6ln@wittgenstein
I explained why d_delete()+d_put() only is wrong and how to fix it.

2021-12-11 22:31:59

by Stefan Berger

[permalink] [raw]
Subject: Re: [PATCH v6 12/17] securityfs: Extend securityfs with namespacing support


On 12/11/21 05:50, Christian Brauner wrote:
> On Fri, Dec 10, 2021 at 02:47:31PM -0500, Stefan Berger wrote:
>> Extend 'securityfs' for support of IMA namespacing so that each
>> IMA (user) namespace can have its own front-end for showing the currently
>> active policy, the measurement list, number of violations and so on.
>>
>> Drop the addition dentry reference and simplify cleanup to work without
>> the additional reference. This enables simple cleanup of dentries upon
>> umount.
>>
>> Signed-off-by: Stefan Berger <[email protected]>
>> Signed-off-by: James Bottomley <[email protected]>
>> ---
>> security/inode.c | 14 ++++----------
>> 1 file changed, 4 insertions(+), 10 deletions(-)
>>
>> diff --git a/security/inode.c b/security/inode.c
>> index fee01ff4d831..c77ae8ecc464 100644
>> --- a/security/inode.c
>> +++ b/security/inode.c
>> @@ -54,7 +54,7 @@ static int securityfs_fill_super(struct super_block *sb, struct fs_context *fc)
>>
>> static int securityfs_get_tree(struct fs_context *fc)
>> {
>> - return get_tree_single(fc, securityfs_fill_super);
>> + return get_tree_keyed(fc, securityfs_fill_super, fc->user_ns);
>> }
>>
>> static const struct fs_context_operations securityfs_context_ops = {
>> @@ -72,6 +72,7 @@ static struct file_system_type fs_type = {
>> .name = "securityfs",
>> .init_fs_context = securityfs_init_fs_context,
>> .kill_sb = kill_litter_super,
>> + .fs_flags = FS_USERNS_MOUNT,
>> };
>>
>> /**
>> @@ -168,7 +169,6 @@ static struct dentry *securityfs_create_dentry(const char *name, umode_t mode,
>> inode->i_fop = fops;
>> }
>> d_instantiate(dentry, inode);
>> - dget(dentry);
>> inode_unlock(dir);
>> return dentry;
>>
>> @@ -306,23 +306,17 @@ EXPORT_SYMBOL_GPL(securityfs_create_symlink);
>> void securityfs_remove(struct dentry *dentry)
>> {
>> struct user_namespace *ns;
>> - struct inode *dir;
>>
>> if (!dentry || IS_ERR(dentry))
>> return;
>>
>> ns = dentry->d_sb->s_user_ns;
>>
>> - dir = d_inode(dentry->d_parent);
>> - inode_lock(dir);
>> if (simple_positive(dentry)) {
>> - if (d_is_dir(dentry))
>> - simple_rmdir(dir, dentry);
>> - else
>> - simple_unlink(dir, dentry);
>> + d_delete(dentry);
>> dput(dentry);
> In
> https://lore.kernel.org/lkml/20211210114934.tacjnwryihrsx6ln@wittgenstein
> I explained why d_delete()+d_put() only is wrong and how to fix it.


Fixed for v7.