linux-kernel - Re: [RFC PATCH v2 2/3] fs: define a firmware security filesystem named fwsecurityfs

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 for Android: free password hash cracker in your pocket

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

Date:   Thu, 23 Jun 2022 09:23:19 -0400
From:   James Bottomley <James.Bottomley@...senPartnership.com>
To:     Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
        Nayna Jain <nayna@...ux.ibm.com>
Cc:     linuxppc-dev@...ts.ozlabs.org, linux-fsdevel@...r.kernel.org,
        linux-efi@...r.kernel.org,
        linux-security-module <linux-security-module@...r.kernel.org>,
        linux-kernel@...r.kernel.org,
        Michael Ellerman <mpe@...erman.id.au>,
        Dov Murik <dovmurik@...ux.ibm.com>,
        George Wilson <gcwilson@...ux.ibm.com>, gjoyce@....com,
        Matthew Garrett <mjg59@...f.ucam.org>,
        Dave Hansen <dave.hansen@...el.com>,
        Benjamin Herrenschmidt <benh@...nel.crashing.org>,
        Paul Mackerras <paulus@...ba.org>
Subject: Re: [RFC PATCH v2 2/3] fs: define a firmware security filesystem
 named fwsecurityfs

On Thu, 2022-06-23 at 10:54 +0200, Greg Kroah-Hartman wrote:
[...]
> > diff --git a/fs/fwsecurityfs/inode.c b/fs/fwsecurityfs/inode.c
> > new file mode 100644
> > index 000000000000..5d06dc0de059
> > --- /dev/null
> > +++ b/fs/fwsecurityfs/inode.c
> > @@ -0,0 +1,159 @@
> > +// SPDX-License-Identifier: GPL-2.0-only
> > +/*
> > + * Copyright (C) 2022 IBM Corporation
> > + * Author: Nayna Jain <nayna@...ux.ibm.com>
> > + */
> > +
> > +#include <linux/sysfs.h>
> > +#include <linux/kobject.h>
> > +#include <linux/fs.h>
> > +#include <linux/fs_context.h>
> > +#include <linux/mount.h>
> > +#include <linux/pagemap.h>
> > +#include <linux/init.h>
> > +#include <linux/namei.h>
> > +#include <linux/security.h>
> > +#include <linux/lsm_hooks.h>
> > +#include <linux/magic.h>
> > +#include <linux/ctype.h>
> > +#include <linux/fwsecurityfs.h>
> > +
> > +#include "internal.h"
> > +
> > +int fwsecurityfs_remove_file(struct dentry *dentry)
> > +{
> > +	drop_nlink(d_inode(dentry));
> > +	dput(dentry);
> > +	return 0;
> > +};
> > +EXPORT_SYMBOL_GPL(fwsecurityfs_remove_file);
> > +
> > +int fwsecurityfs_create_file(const char *name, umode_t mode,
> > +					u16 filesize, struct dentry
> > *parent,
> > +					struct dentry *dentry,
> > +					const struct file_operations
> > *fops)
> > +{
> > +	struct inode *inode;
> > +	int error;
> > +	struct inode *dir;
> > +
> > +	if (!parent)
> > +		return -EINVAL;
> > +
> > +	dir = d_inode(parent);
> > +	pr_debug("securityfs: creating file '%s'\n", name);
> 
> Did you forget to call simple_pin_fs() here or anywhere else?
> 
> And this can be just one function with the directory creation file,
> just check the mode and you will be fine.  Look at securityfs as an
> example of how to make this simpler.

Actually, before you go down this route can you consider the namespace
ramifications.  In fact we're just having to rework securityfs to pull
out all the simple_pin_... calls because simple_pin_... is completely
inimical to namespaces.

The first thing to consider is if you simply use securityfs you'll
inherit all the simple_pin_... removal work and be namespace ready.  It
could be that creating a new filesystem that can't be namespaced is the
right thing to do here, but at least ask the question: would we ever
want any of these files to be presented selectively inside containers? 
If the answer is "yes" then simple_pin_... is the wrong interface.

James