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

[Jason Wang]

On Tue, Feb 25, 2025 at 8:31 PM Michael S. Tsirkin <mst@xxxxxxxxxx> wrote:
>
> 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@xxxxxxxxxx> 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@xxxxxxxxxx>
> > >
> > >
> > > 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.

Can we simply start from CAP_SYS_ADMIN?

Thanks

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