linux-kernel - Re: [RFC PATCH 14/19] famfs_fuse: GET_DAXDEV message and daxdev

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]

Message-ID: <6f22k2r6uu4rimplfdna7farx3o2vfwp3korye54tfezemfl3q@hcngav32igrt>
Date: Mon, 21 Apr 2025 15:57:57 -0500
From: John Groves <John@...ves.net>
To: Randy Dunlap <rdunlap@...radead.org>
Cc: Dan Williams <dan.j.williams@...el.com>, 
	Miklos Szeredi <miklos@...redb.hu>, Bernd Schubert <bschubert@....com>, 
	John Groves <jgroves@...ron.com>, Jonathan Corbet <corbet@....net>, 
	Vishal Verma <vishal.l.verma@...el.com>, Dave Jiang <dave.jiang@...el.com>, 
	Matthew Wilcox <willy@...radead.org>, Jan Kara <jack@...e.cz>, 
	Alexander Viro <viro@...iv.linux.org.uk>, Christian Brauner <brauner@...nel.org>, 
	"Darrick J . Wong" <djwong@...nel.org>, Luis Henriques <luis@...lia.com>, 
	Jeff Layton <jlayton@...nel.org>, Kent Overstreet <kent.overstreet@...ux.dev>, 
	Petr Vorel <pvorel@...e.cz>, Brian Foster <bfoster@...hat.com>, linux-doc@...r.kernel.org, 
	linux-kernel@...r.kernel.org, nvdimm@...ts.linux.dev, linux-cxl@...r.kernel.org, 
	linux-fsdevel@...r.kernel.org, Amir Goldstein <amir73il@...il.com>, 
	Jonathan Cameron <Jonathan.Cameron@...wei.com>, Stefan Hajnoczi <shajnocz@...hat.com>, 
	Joanne Koong <joannelkoong@...il.com>, Josef Bacik <josef@...icpanda.com>, 
	Aravind Ramesh <arramesh@...ron.com>, Ajay Joshi <ajayjoshi@...ron.com>
Subject: Re: [RFC PATCH 14/19] famfs_fuse: GET_DAXDEV message and daxdev_table

On 25/04/20 08:43PM, Randy Dunlap wrote:
> Hi,

Hi Randy - thanks for the review!

> 
> On 4/20/25 6:33 PM, John Groves wrote:
> > * The new GET_DAXDEV message/response is enabled
> > * The command it triggered by the update_daxdev_table() call, if there
> >   are any daxdevs in the subject fmap that are not represented in the
> >   daxdev_dable yet.
> > 
> > Signed-off-by: John Groves <john@...ves.net>
> > ---
> >  fs/fuse/famfs.c           | 281 ++++++++++++++++++++++++++++++++++++--
> >  fs/fuse/famfs_kfmap.h     |  23 ++++
> >  fs/fuse/fuse_i.h          |   4 +
> >  fs/fuse/inode.c           |   2 +
> >  fs/namei.c                |   1 +
> >  include/uapi/linux/fuse.h |  15 ++
> >  6 files changed, 316 insertions(+), 10 deletions(-)
> > 
> > diff --git a/fs/fuse/famfs.c b/fs/fuse/famfs.c
> > index e62c047d0950..2e182cb7d7c9 100644
> > --- a/fs/fuse/famfs.c
> > +++ b/fs/fuse/famfs.c
> > @@ -20,6 +20,250 @@
> >  #include "famfs_kfmap.h"
> >  #include "fuse_i.h"
> >  
> > +/*
> > + * famfs_teardown()
> > + *
> > + * Deallocate famfs metadata for a fuse_conn
> > + */
> > +void
> > +famfs_teardown(struct fuse_conn *fc)
> 
> Is this function formatting prevalent in fuse?
> It's a bit different from most Linux.
> (many locations throughout the patch set)

I'll check and clean it up if not; function names beginning in column 1 is a
"thing", but I'll normalize to nearby standards.

> 
> > +{
> > +	struct famfs_dax_devlist *devlist = fc->dax_devlist;
> > +	int i;
> > +
> > +	fc->dax_devlist = NULL;
> > +
> > +	if (!devlist)
> > +		return;
> > +
> > +	if (!devlist->devlist)
> > +		goto out;
> > +
> > +	/* Close & release all the daxdevs in our table */
> > +	for (i = 0; i < devlist->nslots; i++) {
> > +		if (devlist->devlist[i].valid && devlist->devlist[i].devp)
> > +			fs_put_dax(devlist->devlist[i].devp, fc);
> > +	}
> > +	kfree(devlist->devlist);
> > +
> > +out:
> > +	kfree(devlist);
> > +}
> > +
> > +static int
> > +famfs_verify_daxdev(const char *pathname, dev_t *devno)
> > +{
> > +	struct inode *inode;
> > +	struct path path;
> > +	int err;
> > +
> > +	if (!pathname || !*pathname)
> > +		return -EINVAL;
> > +
> > +	err = kern_path(pathname, LOOKUP_FOLLOW, &path);
> > +	if (err)
> > +		return err;
> > +
> > +	inode = d_backing_inode(path.dentry);
> > +	if (!S_ISCHR(inode->i_mode)) {
> > +		err = -EINVAL;
> > +		goto out_path_put;
> > +	}
> > +
> > +	if (!may_open_dev(&path)) { /* had to export this */
> > +		err = -EACCES;
> > +		goto out_path_put;
> > +	}
> > +
> > +	*devno = inode->i_rdev;
> > +
> > +out_path_put:
> > +	path_put(&path);
> > +	return err;
> > +}
> > +
> > +/**
> > + * famfs_fuse_get_daxdev()
> 
> Missing " - <short function description>"
> but then it's a static function, so kernel-doc is not required.
> It's up to you, but please use full kernel-doc notation if using kernel-doc.

Thank you - and sorry for being a bit sloppy on this stuff. I'm caching fixes
for all your comments along these lines into a branch for the next version of
the series.

Snipping the rest, but will address it all.

Thanks,
John