| Message ID | 20240506185713.58678-1-bfoster@redhat.com (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | [v2] virtiofs: use string format specifier for sysfs tag | expand |
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
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>
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
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);
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(-)