On Donnerstag, 27. Juni 2019 19:26:22 CEST Greg Kurz wrote:
> On Wed, 26 Jun 2019 20:30:41 +0200
>
> Christian Schoenebeck via Qemu-devel <qemu-devel@nongnu.org> wrote:
> > The QID path should uniquely identify a file. However, the
> > inode of a file is currently used as the QID path, which
> > on its own only uniquely identifies wiles within a device.
>
> s/wile/files
Ah right. :)
> > Here we track the device hosting the 9pfs share, in order
> > to prevent security issues with QID path collisions from
> > other devices.
> >
> > Signed-off-by: Antonios Motakis <antonios.mota...@huawei.com>
>
> You should mention here the changes you made to the original patch.
Got it. Will do for the other cases as well of course.
> > -static void stat_to_qid(const struct stat *stbuf, V9fsQID *qidp)
> > +static int stat_to_qid(V9fsPDU *pdu, const struct stat *stbuf, V9fsQID
> > *qidp)>
> > {
> >
> > size_t size;
> >
> > + if (pdu->s->dev_id == 0) {
> > + pdu->s->dev_id = stbuf->st_dev;
>
> st_dev should be captured in v9fs_device_realize_common() since we
> lstat() the root there, instead of every request doing the check.
Ok.
> > + } else if (pdu->s->dev_id != stbuf->st_dev) {
> > + error_report_once(
> > + "9p: Multiple devices detected in same VirtFS export. "
> > + "You must use a separate export for each device."
> > + );
> > + return -ENOSYS;
>
> This error is likely to end up as the return value of a
> syscall in the guest and -ENOSYS usually means the syscall
> isn't implemented, which is obviously not the case. Maybe
> return -EPERM instead ?
I would rather suggest -ENODEV. The entire device of the requested file/dir is
not available on guest.
-EPERM IMO rather motivates users looking for file system permission settings
on individual files intead, and probably not checking the host's logs for the
detailled error message.
> > @@ -3633,6 +3674,8 @@ int v9fs_device_realize_common(V9fsState *s, const
> > V9fsTransport *t,>
> > goto out;
> >
> > }
> >
> > + s->dev_id = 0;
> > +
>
> Set it to stat->st_dev after lstat() was called later in this function.
I guesst you mean "earlier" not "later". The lstat() call is just before that
dev_id initalization line. But in general your suggestion makes sense of
course.
Best regards,
Christian Schoenebeck
