Re: [PATCH] virtiofs: Export filesystem tags through sysfs

[Greg KH <gregkh@linuxfoundation.org>]

On Fri, Jan 05, 2024 at 03:44:55PM -0500, Vivek Goyal wrote:
> On Sat, Nov 11, 2023 at 06:53:40AM -0500, Greg KH wrote:
> > On Thu, Oct 05, 2023 at 04:30:30PM -0400, Vivek Goyal wrote:
> > > virtiofs filesystem is mounted using a "tag" which is exported by the
> > > virtiofs device. virtiofs driver knows about all the available tags but
> > > these are not exported to user space.
> > > 
> > > People have asked these tags to be exported to user space. Most recently
> > > Lennart Poettering has asked for it as he wants to scan the tags and mount
> > > virtiofs automatically in certain cases.
> > > 
> > > https://gitlab.com/virtio-fs/virtiofsd/-/issues/128
> > > 
> > > This patch exports tags through sysfs. One tag is associated with each
> > > virtiofs device. A new "tag" file appears under virtiofs device dir.
> > > Actual filesystem tag can be obtained by reading this "tag" file.
> > > 
> > > For example, if a virtiofs device exports tag "myfs", a new file "tag"
> > > will show up here.
> > > 
> > > /sys/bus/virtio/devices/virtio<N>/tag
> > > 
> > > # cat /sys/bus/virtio/devices/virtio<N>/tag
> > > myfs
> > > 
> > > Signed-off-by: Vivek Goyal <vgoyal@redhat.com>
> > > ---
> > >  fs/fuse/virtio_fs.c | 34 ++++++++++++++++++++++++++++++++++
> > >  1 file changed, 34 insertions(+)
> > > 
> > > diff --git a/fs/fuse/virtio_fs.c b/fs/fuse/virtio_fs.c
> > > index 5f1be1da92ce..a5b11e18f331 100644
> > > --- a/fs/fuse/virtio_fs.c
> > > +++ b/fs/fuse/virtio_fs.c
> > > @@ -107,6 +107,21 @@ static const struct fs_parameter_spec virtio_fs_parameters[] = {
> > >  	{}
> > >  };
> > >  
> > > +/* Forward Declarations */
> > > +static void virtio_fs_stop_all_queues(struct virtio_fs *fs);
> > > +
> > > +/* sysfs related */
> > > +static ssize_t tag_show(struct device *dev, struct device_attribute *attr,
> > > +			char *buf)
> > > +{
> > > +	struct virtio_device *vdev = container_of(dev, struct virtio_device,
> > > +						  dev);
> > > +	struct virtio_fs *fs = vdev->priv;
> > > +
> > > +	return sysfs_emit(buf, "%s", fs->tag);
> > > +}
> > > +static DEVICE_ATTR_RO(tag);
> > > +
> > >  static int virtio_fs_parse_param(struct fs_context *fsc,
> > >  				 struct fs_parameter *param)
> > >  {
> > > @@ -265,6 +280,15 @@ static int virtio_fs_add_instance(struct virtio_fs *fs)
> > >  	return 0;
> > >  }
> > >  
> > > +static void virtio_fs_remove_instance(struct virtio_fs *fs)
> > > +{
> > > +	mutex_lock(&virtio_fs_mutex);
> > > +	list_del_init(&fs->list);
> > > +	virtio_fs_stop_all_queues(fs);
> > > +	virtio_fs_drain_all_queues_locked(fs);
> > > +	mutex_unlock(&virtio_fs_mutex);
> > > +}
> > > +
> > >  /* Return the virtio_fs with a given tag, or NULL */
> > >  static struct virtio_fs *virtio_fs_find_instance(const char *tag)
> > >  {
> > > @@ -891,8 +915,15 @@ static int virtio_fs_probe(struct virtio_device *vdev)
> > >  	if (ret < 0)
> > >  		goto out_vqs;
> > >  
> > > +	/* Export tag through sysfs */
> > > +	ret = device_create_file(&vdev->dev, &dev_attr_tag);
> > > +	if (ret < 0)
> > > +		goto out_sysfs_attr;
> > 
> > You just raced with userspace and lost :(
> > 
> > Please use default groups for your device/bus, that is what they are
> > there for.
> 
> Hi Greg,
> 
> Getting back to this thread after a long time. Sorry, got busy in other
> things.
> 
> Trying to understand the races with user space. IIUC, your primary concern
> is that kobject has already been published to the user space and I am
> trying to add/remove attributes to the object after the fact and that
> makes it racy?

Yes.

> And suggestion is to first fully create kobject (including attrs) before
> publishing it to user space. Do I understand it correctly or I missed
> the point completely?

Use the default groups pointer in the device instead which makes the
driver core create/remove the files automatically for you.  Here's
something I wrote over a decade ago as to how to prevent this:
	http://www.kroah.com/log/blog/2013/06/26/how-to-create-a-sysfs-file-correctly/

thanks,

greg k-h