Skip to content

Commit

Permalink
9p: Treat multiple devices on one export as an error
Browse files Browse the repository at this point in the history 
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 files within a device.
Here we track the device hosting the 9pfs share, in order
to prevent security issues with QID path collisions from
other devices.

We only print a warning for now but a subsequent patch will
allow users to have finer control over the desired behaviour.
Failing the I/O will be one the proposed behaviour, so we
also change stat_to_qid() to return an error here in order to
keep other patches simpler.

Signed-off-by: Antonios Motakis <antonios.motakis@huawei.com>
[CS: - Assign dev_id to export root's device already in
       v9fs_device_realize_common(), not postponed in
       stat_to_qid().
     - error_report_once() if more than one device was
       shared by export.
     - Return -ENODEV instead of -ENOSYS in stat_to_qid().
     - Fixed typo in log comment. ]
Signed-off-by: Christian Schoenebeck <qemu_oss@crudebyte.com>
[groug, changed to warning, updated message and changelog]
Signed-off-by: Greg Kurz <groug@kaod.org>
  • Loading branch information
@tvelocity @gkurz
tvelocity authored and gkurz committed Oct 10, 2019
1 parent ea52cdd commit 3b5ee9e
Show file tree
Hide file tree
Showing 2 changed files with 57 additions and 14 deletions.
70 changes: 56 additions & 14 deletions hw/9pfs/9p.c
View file
Expand Up @@ -573,10 +573,19 @@ static void coroutine_fn virtfs_reset(V9fsPDU *pdu)
P9_STAT_MODE_SOCKET)

/* This is the algorithm from ufs in spfs */
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 != stbuf->st_dev) {
warn_report_once(
"9p: Multiple devices detected in same VirtFS export, "
"which might lead to file ID collisions and severe "
"misbehaviours on guest! You should use a separate "
"export for each device shared from host."
);
}

memset(&qidp->path, 0, sizeof(qidp->path));
size = MIN(sizeof(stbuf->st_ino), sizeof(qidp->path));
memcpy(&qidp->path, &stbuf->st_ino, size);
Expand All @@ -588,6 +597,8 @@ static void stat_to_qid(const struct stat *stbuf, V9fsQID *qidp)
if (S_ISLNK(stbuf->st_mode)) {
qidp->type |= P9_QID_TYPE_SYMLINK;
}

return 0;
}

static int coroutine_fn fid_to_qid(V9fsPDU *pdu, V9fsFidState *fidp,
Expand All @@ -600,7 +611,10 @@ static int coroutine_fn fid_to_qid(V9fsPDU *pdu, V9fsFidState *fidp,
if (err < 0) {
return err;
}
stat_to_qid(&stbuf, qidp);
err = stat_to_qid(pdu, &stbuf, qidp);
if (err < 0) {
return err;
}
return 0;
}

Expand Down Expand Up @@ -831,7 +845,10 @@ static int coroutine_fn stat_to_v9stat(V9fsPDU *pdu, V9fsPath *path,

memset(v9stat, 0, sizeof(*v9stat));

stat_to_qid(stbuf, &v9stat->qid);
err = stat_to_qid(pdu, stbuf, &v9stat->qid);
if (err < 0) {
return err;
}
v9stat->mode = stat_to_v9mode(stbuf);
v9stat->atime = stbuf->st_atime;
v9stat->mtime = stbuf->st_mtime;
Expand Down Expand Up @@ -892,7 +909,7 @@ static int coroutine_fn stat_to_v9stat(V9fsPDU *pdu, V9fsPath *path,
#define P9_STATS_ALL 0x00003fffULL /* Mask for All fields above */


static void stat_to_v9stat_dotl(V9fsState *s, const struct stat *stbuf,
static int stat_to_v9stat_dotl(V9fsPDU *pdu, const struct stat *stbuf,
V9fsStatDotl *v9lstat)
{
memset(v9lstat, 0, sizeof(*v9lstat));
Expand All @@ -914,7 +931,7 @@ static void stat_to_v9stat_dotl(V9fsState *s, const struct stat *stbuf,
/* Currently we only support BASIC fields in stat */
v9lstat->st_result_mask = P9_STATS_BASIC;

stat_to_qid(stbuf, &v9lstat->qid);
return stat_to_qid(pdu, stbuf, &v9lstat->qid);
}

static void print_sg(struct iovec *sg, int cnt)
Expand Down Expand Up @@ -1116,7 +1133,6 @@ static void coroutine_fn v9fs_getattr(void *opaque)
uint64_t request_mask;
V9fsStatDotl v9stat_dotl;
V9fsPDU *pdu = opaque;
V9fsState *s = pdu->s;

retval = pdu_unmarshal(pdu, offset, "dq", &fid, &request_mask);
if (retval < 0) {
Expand All @@ -1137,7 +1153,10 @@ static void coroutine_fn v9fs_getattr(void *opaque)
if (retval < 0) {
goto out;
}
stat_to_v9stat_dotl(s, &stbuf, &v9stat_dotl);
retval = stat_to_v9stat_dotl(pdu, &stbuf, &v9stat_dotl);
if (retval < 0) {
goto out;
}

/* fill st_gen if requested and supported by underlying fs */
if (request_mask & P9_STATS_GEN) {
Expand Down Expand Up @@ -1382,7 +1401,10 @@ static void coroutine_fn v9fs_walk(void *opaque)
if (err < 0) {
goto out;
}
stat_to_qid(&stbuf, &qid);
err = stat_to_qid(pdu, &stbuf, &qid);
if (err < 0) {
goto out;
}
v9fs_path_copy(&dpath, &path);
}
memcpy(&qids[name_idx], &qid, sizeof(qid));
Expand Down Expand Up @@ -1484,7 +1506,10 @@ static void coroutine_fn v9fs_open(void *opaque)
if (err < 0) {
goto out;
}
stat_to_qid(&stbuf, &qid);
err = stat_to_qid(pdu, &stbuf, &qid);
if (err < 0) {
goto out;
}
if (S_ISDIR(stbuf.st_mode)) {
err = v9fs_co_opendir(pdu, fidp);
if (err < 0) {
Expand Down Expand Up @@ -1594,7 +1619,10 @@ static void coroutine_fn v9fs_lcreate(void *opaque)
fidp->flags |= FID_NON_RECLAIMABLE;
}
iounit = get_iounit(pdu, &fidp->path);
stat_to_qid(&stbuf, &qid);
err = stat_to_qid(pdu, &stbuf, &qid);
if (err < 0) {
goto out;
}
err = pdu_marshal(pdu, offset, "Qd", &qid, iounit);
if (err < 0) {
goto out;
Expand Down Expand Up @@ -2328,7 +2356,10 @@ static void coroutine_fn v9fs_create(void *opaque)
}
}
iounit = get_iounit(pdu, &fidp->path);
stat_to_qid(&stbuf, &qid);
err = stat_to_qid(pdu, &stbuf, &qid);
if (err < 0) {
goto out;
}
err = pdu_marshal(pdu, offset, "Qd", &qid, iounit);
if (err < 0) {
goto out;
Expand Down Expand Up @@ -2385,7 +2416,10 @@ static void coroutine_fn v9fs_symlink(void *opaque)
if (err < 0) {
goto out;
}
stat_to_qid(&stbuf, &qid);
err = stat_to_qid(pdu, &stbuf, &qid);
if (err < 0) {
goto out;
}
err = pdu_marshal(pdu, offset, "Q", &qid);
if (err < 0) {
goto out;
Expand Down Expand Up @@ -3065,7 +3099,10 @@ static void coroutine_fn v9fs_mknod(void *opaque)
if (err < 0) {
goto out;
}
stat_to_qid(&stbuf, &qid);
err = stat_to_qid(pdu, &stbuf, &qid);
if (err < 0) {
goto out;
}
err = pdu_marshal(pdu, offset, "Q", &qid);
if (err < 0) {
goto out;
Expand Down Expand Up @@ -3223,7 +3260,10 @@ static void coroutine_fn v9fs_mkdir(void *opaque)
if (err < 0) {
goto out;
}
stat_to_qid(&stbuf, &qid);
err = stat_to_qid(pdu, &stbuf, &qid);
if (err < 0) {
goto out;
}
err = pdu_marshal(pdu, offset, "Q", &qid);
if (err < 0) {
goto out;
Expand Down Expand Up @@ -3634,6 +3674,8 @@ int v9fs_device_realize_common(V9fsState *s, const V9fsTransport *t,
goto out;
}

s->dev_id = stat.st_dev;

s->ctx.fst = &fse->fst;
fsdev_throttle_init(s->ctx.fst);

Expand Down
1 change: 1 addition & 0 deletions hw/9pfs/9p.h
View file
Expand Up @@ -256,6 +256,7 @@ struct V9fsState
Error *migration_blocker;
V9fsConf fsconf;
V9fsQID root_qid;
dev_t dev_id;
};

/* 9p2000.L open flags */
Expand Down

0 comments on commit 3b5ee9e

Please sign in to comment.