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

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

[Brian Foster]

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);
 }
 
 static struct kobj_attribute virtio_fs_tag_attr = __ATTR_RO(tag);
-- 
2.44.0

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

[Stefan Hajnoczi]

[-- Attachment #1: Type: text/plain, Size: 1291 bytes --]

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.

Thank you,
Stefan

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

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

[Brian Foster]

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

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

[Stefan Hajnoczi]

[-- Attachment #1: Type: text/plain, Size: 562 bytes --]

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(-)

Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]