Re: [PATCH] vduse: add virtio_fs to allowed dev id

["Michael S. Tsirkin" <mst@redhat.com>]

On Tue, Feb 25, 2025 at 01:17:02PM +0100, Eugenio Perez Martin wrote:
> On Mon, Feb 24, 2025 at 10:51 PM Michael S. Tsirkin <mst@redhat.com> wrote:
> >
> > On Tue, Jan 21, 2025 at 11:33:46AM +0100, Eugenio Pérez wrote:
> > > A VDUSE device that implements virtiofs device works fine just by
> > > adding the device id to the whitelist.
> > >
> > > Signed-off-by: Eugenio Pérez <eperezma@redhat.com>
> >
> >
> > OK, but the commit log really should say why
> > you are doing this.
> 
> Sure I can expand on the motivation.
> 
> Something like "Allowing VDUSE FS type allows to build filesystems
> that run in userspace and can be presented transparently to the host
> and the guest. After modifying userland's libfuse, this allows to
> expose a good amount to already available userland FS through vDPA."
> 
> I'd add using the high performance virtio protocol but I still need to
> do more tests for this TBH.
> 
> > And also why is it safe.
> >
> 
> Can you expand on the scenarios you think this is insecure? While I
> understand it's security sensitive, you already need root to perform
> vdpa device operations. Is FS different from net or block?
> 
> Thanks!

I did not say it was insecure, just that you need to explain the
security considerations in the commit log.
The issue is that when one gave access to vdpa user device previously
it would only allow mounting blk now a filesystem.

Net is different, it is gated by CAP_NET_ADMIN.

When net was introduced, selinux was there initially then it
was deferred and never surfaced.
Maybe we should revive it so it is possible to control which
devices can be created in a granular way.

> > > ---
> > >  drivers/vdpa/vdpa_user/vduse_dev.c | 1 +
> > >  1 file changed, 1 insertion(+)
> > >
> > > diff --git a/drivers/vdpa/vdpa_user/vduse_dev.c b/drivers/vdpa/vdpa_user/vduse_dev.c
> > > index 7ae99691efdf..6a9a37351310 100644
> > > --- a/drivers/vdpa/vdpa_user/vduse_dev.c
> > > +++ b/drivers/vdpa/vdpa_user/vduse_dev.c
> > > @@ -144,6 +144,7 @@ static struct workqueue_struct *vduse_irq_bound_wq;
> > >  static u32 allowed_device_id[] = {
> > >       VIRTIO_ID_BLOCK,
> > >       VIRTIO_ID_NET,
> > > +     VIRTIO_ID_FS,
> > >  };
> > >
> > >  static inline struct vduse_dev *vdpa_to_vduse(struct vdpa_device *vdpa)
> > > --
> > > 2.48.1
> >