9p: Treat multiple devices on one export as an error
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>
diff --git a/hw/9pfs/9p.c b/hw/9pfs/9p.c
index ba1ab92..5a895ae 100644
--- a/hw/9pfs/9p.c
+++ b/hw/9pfs/9p.c
@@ -573,10 +573,19 @@
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);
@@ -588,6 +597,8 @@
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,
@@ -600,7 +611,10 @@
if (err < 0) {
return err;
}
- stat_to_qid(&stbuf, qidp);
+ err = stat_to_qid(pdu, &stbuf, qidp);
+ if (err < 0) {
+ return err;
+ }
return 0;
}
@@ -831,7 +845,10 @@
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;
@@ -892,7 +909,7 @@
#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));
@@ -914,7 +931,7 @@
/* 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)
@@ -1116,7 +1133,6 @@
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) {
@@ -1137,7 +1153,10 @@
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) {
@@ -1382,7 +1401,10 @@
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));
@@ -1484,7 +1506,10 @@
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) {
@@ -1594,7 +1619,10 @@
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;
@@ -2328,7 +2356,10 @@
}
}
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;
@@ -2385,7 +2416,10 @@
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;
@@ -3065,7 +3099,10 @@
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;
@@ -3223,7 +3260,10 @@
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;
@@ -3634,6 +3674,8 @@
goto out;
}
+ s->dev_id = stat.st_dev;
+
s->ctx.fst = &fse->fst;
fsdev_throttle_init(s->ctx.fst);
