Re: [PATCH v2] virtiofs: use string format specifier for sysfs tag

[Brian Foster <bfoster@redhat.com>]

On Tue, May 07, 2024 at 09:54:19AM -0400, Stefan Hajnoczi wrote:
> On Mon, May 06, 2024 at 02:57:13PM -0400, Brian Foster wrote:
> > The existing emit call is a vector for format string injection. Use
> > the string format specifier to avoid this problem.
> > 
> > Reported-by: Stefan Hajnoczi <stefanha@redhat.com>
> > Signed-off-by: Brian Foster <bfoster@redhat.com>
> > ---
> > 
> > v2:
> > - Drop newline.
> > v1: https://lore.kernel.org/linux-fsdevel/20240425104400.30222-1-bfoster@redhat.com/
> > 
> >  fs/fuse/virtio_fs.c | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> > 
> > diff --git a/fs/fuse/virtio_fs.c b/fs/fuse/virtio_fs.c
> > index 322af827a232..d5cb300367ed 100644
> > --- a/fs/fuse/virtio_fs.c
> > +++ b/fs/fuse/virtio_fs.c
> > @@ -170,7 +170,7 @@ static ssize_t tag_show(struct kobject *kobj,
> >  {
> >  	struct virtio_fs *fs = container_of(kobj, struct virtio_fs, kobj);
> >  
> > -	return sysfs_emit(buf, fs->tag);
> > +	return sysfs_emit(buf, "%s", fs->tag);
> >  }
> 
> Miklos: Would it be possible to change the format string to "%s\n" (with
> a newline) in this patch and merged for v6.9?
> 
> v6.9 will be the first kernel release with this new sysfs attr and I'd
> like to get the formatting right. Once a kernel is released I would
> rather not change the sysfs attr's format to avoid breaking userspace,
> hence the urgency.
> 

It might be worth including the following tag in this as well:

Fixes: a8f62f50b4e4 ("virtiofs: export filesystem tags through sysfs")

... re: the discussion on v1.

I'd also advocate for including the newline either way, but again I
defer to Stefan if he feels strongly about it. FWIW, if we do go that
route I can also send a v3 with the tag and combined v1/v2 commit log if
that is helpful.

Brian

> Thank you,
> Stefan