On Tue, Apr 30, 2024 at 01:34:31PM -0400, Stefan Hajnoczi wrote:
> On Thu, Apr 25, 2024 at 06:44:00AM -0400, Brian Foster wrote:
> > The internal tag string doesn't contain a newline. Append one when
> > emitting the tag via sysfs.
> >
> > Signed-off-by: Brian Foster <bfoster@xxxxxxxxxx>
> > ---
> >
> > Hi all,
> >
> > I just noticed this and it seemed a little odd to me compared to typical
> > sysfs output, but maybe it was intentional..? Easy enough to send a
> > patch either way.. thoughts?
>
> Hi Brian,
> Orthogonal to the newline issue, sysfs_emit(buf, "%s", fs->tag) is
> needed to prevent format string injection. Please mention this in the
> commit description. I'm afraid I introduced that bug, sorry!
>
Hi Stefan,
Ah, thanks. That hadn't crossed my mind.
> Regarding newline, I'm concerned that adding a newline might break
> existing programs. Unless there is a concrete need to have the newline,
> I would keep things as they are.
>
Not sure I follow the concern.. wasn't this interface just added? Did
you have certain userspace tools in mind?
FWIW, my reason for posting this was that the first thing I did to try
out this functionality was basically a 'cat /sys/fs/virtiofs/*/tag' to
see what fs' were attached to my vm, and then I got a single line
concatenation of every virtiofs tag and found that pretty annoying. ;)
I don't know that is a concrete need for the newline, but I still find
the current behavior kind of odd. That said, I'll defer to you guys if
you'd prefer to leave it alone. I just posted a v2 for the format
specifier thing as above and you can decide which patch to take or not..
Brian
> Stefan
>
> >
> > Brian
> >
> > 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..bb3e941b9503 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\n", fs->tag);
> > }
> >
> > static struct kobj_attribute virtio_fs_tag_attr = __ATTR_RO(tag);
> > --
> > 2.44.0
> >
