9pfs: fix 'Twalk' protocol violation

Actual fix is patch 5, whereas patch 4 being preparatory, all other
 patches are test cases to guard this Twalk issue.
 -----BEGIN PGP SIGNATURE-----
 
 iQJLBAABCgA1FiEEltjREM96+AhPiFkBNMK1h2Wkc5UFAmKrDSAXHHFlbXVfb3Nz
 QGNydWRlYnl0ZS5jb20ACgkQNMK1h2Wkc5VgoQ//bA/lXYa6hds4f73+opq7iiJ/
 88gnJO8uPctNWXJ5f6ufXcTFtC99QRcl97jgSQhSIUdaZCfcpg7Pq3fONc060cMt
 MNxi5Da31Fq7xz4UhSQHgWlgAfomfClYoBSOtrrxjVbXChA2rB7FXhD9aewimUtt
 TlolXdJuPbGR3F6H0glN1itij12Ay5c0DMqFPy5npYlzjNhxmPb8QgFZ8E+lxhcT
 hG+OMmS9O5Mk7WKYWC1Iij7tWm45RbThPEUsfCPt6jIJYQqheOQs0ohJG9wyCZu3
 JUCgSBPG1nNY0hgBJ/X7un7e89BoRw8edwqP+sSigfDf+LquUggqRFgz+joTbfvj
 Prq+1NTDIckDRZF6CDUSkZE3+Gq3qlIhw/2vS+bjYZrk04lP4x8d9JYPSkT3i8xc
 +YT/apDUkT68FjJ6PudfS2j6xRtYt86nOuWuhYukTZ2z5FJ0c9XAJlJX2ZS9Az3n
 AqKFCT+8UE4VYKnAJ61xDdqvAdEmKJUi5YutfuwH+j6sS4peLX0gg8mGlNi7y8JK
 bsqNjE1ve8rkp24DuUoHmivs/m1ogJi9Jxp5IjB4d26MPhgojrxOpaYUVg98QS7d
 os2ES47CSn4KFxqsFMZnZpgzKxIvRQ4C9bBbSClDOffHWHRJub6PCw5F9eCTH4dO
 z/QPJ+smDY7bolF+gSg=
 =3ejn
 -----END PGP SIGNATURE-----

Merge tag 'pull-9p-20220616' of https://github.com/cschoenebeck/qemu into staging

9pfs: fix 'Twalk' protocol violation

Actual fix is patch 5, whereas patch 4 being preparatory, all other
patches are test cases to guard this Twalk issue.

# -----BEGIN PGP SIGNATURE-----
#
# iQJLBAABCgA1FiEEltjREM96+AhPiFkBNMK1h2Wkc5UFAmKrDSAXHHFlbXVfb3Nz
# QGNydWRlYnl0ZS5jb20ACgkQNMK1h2Wkc5VgoQ//bA/lXYa6hds4f73+opq7iiJ/
# 88gnJO8uPctNWXJ5f6ufXcTFtC99QRcl97jgSQhSIUdaZCfcpg7Pq3fONc060cMt
# MNxi5Da31Fq7xz4UhSQHgWlgAfomfClYoBSOtrrxjVbXChA2rB7FXhD9aewimUtt
# TlolXdJuPbGR3F6H0glN1itij12Ay5c0DMqFPy5npYlzjNhxmPb8QgFZ8E+lxhcT
# hG+OMmS9O5Mk7WKYWC1Iij7tWm45RbThPEUsfCPt6jIJYQqheOQs0ohJG9wyCZu3
# JUCgSBPG1nNY0hgBJ/X7un7e89BoRw8edwqP+sSigfDf+LquUggqRFgz+joTbfvj
# Prq+1NTDIckDRZF6CDUSkZE3+Gq3qlIhw/2vS+bjYZrk04lP4x8d9JYPSkT3i8xc
# +YT/apDUkT68FjJ6PudfS2j6xRtYt86nOuWuhYukTZ2z5FJ0c9XAJlJX2ZS9Az3n
# AqKFCT+8UE4VYKnAJ61xDdqvAdEmKJUi5YutfuwH+j6sS4peLX0gg8mGlNi7y8JK
# bsqNjE1ve8rkp24DuUoHmivs/m1ogJi9Jxp5IjB4d26MPhgojrxOpaYUVg98QS7d
# os2ES47CSn4KFxqsFMZnZpgzKxIvRQ4C9bBbSClDOffHWHRJub6PCw5F9eCTH4dO
# z/QPJ+smDY7bolF+gSg=
# =3ejn
# -----END PGP SIGNATURE-----
# gpg: Signature made Thu 16 Jun 2022 03:59:44 AM PDT
# gpg:                using RSA key 96D8D110CF7AF8084F88590134C2B58765A47395
# gpg:                issuer "qemu_oss@crudebyte.com"
# gpg: Good signature from "Christian Schoenebeck <qemu_oss@crudebyte.com>" [unknown]
# gpg: WARNING: This key is not certified with a trusted signature!
# gpg:          There is no indication that the signature belongs to the owner.
# Primary key fingerprint: ECAB 1A45 4014 1413 BA38  4926 30DB 47C3 A012 D5F4
#      Subkey fingerprint: 96D8 D110 CF7A F808 4F88  5901 34C2 B587 65A4 7395

* tag 'pull-9p-20220616' of https://github.com/cschoenebeck/qemu:
  tests/9pfs: check fid being unaffected in fs_walk_2nd_nonexistent
  tests/9pfs: guard recent 'Twalk' behaviour fix
  9pfs: fix 'Twalk' to only send error if no component walked
  9pfs: refactor 'name_idx' -> 'nwalked' in v9fs_walk()
  tests/9pfs: compare QIDs in fs_walk_none() test
  tests/9pfs: Twalk with nwname=0
  tests/9pfs: walk to non-existent dir

Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
This commit is contained in:
Richard Henderson 2022-06-16 09:15:40 -07:00
commit 213fda642d
2 changed files with 238 additions and 28 deletions

View File

@ -1766,9 +1766,9 @@ static bool same_stat_id(const struct stat *a, const struct stat *b)
static void coroutine_fn v9fs_walk(void *opaque)
{
int name_idx;
int name_idx, nwalked;
g_autofree V9fsQID *qids = NULL;
int i, err = 0;
int i, err = 0, any_err = 0;
V9fsPath dpath, path;
P9ARRAY_REF(V9fsPath) pathes = NULL;
uint16_t nwnames;
@ -1834,54 +1834,61 @@ static void coroutine_fn v9fs_walk(void *opaque)
* driver code altogether inside the following block.
*/
v9fs_co_run_in_worker({
nwalked = 0;
if (v9fs_request_cancelled(pdu)) {
err = -EINTR;
any_err |= err = -EINTR;
break;
}
err = s->ops->lstat(&s->ctx, &dpath, &fidst);
if (err < 0) {
err = -errno;
any_err |= err = -errno;
break;
}
stbuf = fidst;
for (name_idx = 0; name_idx < nwnames; name_idx++) {
for (; nwalked < nwnames; nwalked++) {
if (v9fs_request_cancelled(pdu)) {
err = -EINTR;
any_err |= err = -EINTR;
break;
}
if (!same_stat_id(&pdu->s->root_st, &stbuf) ||
strcmp("..", wnames[name_idx].data))
strcmp("..", wnames[nwalked].data))
{
err = s->ops->name_to_path(&s->ctx, &dpath,
wnames[name_idx].data,
&pathes[name_idx]);
wnames[nwalked].data,
&pathes[nwalked]);
if (err < 0) {
err = -errno;
any_err |= err = -errno;
break;
}
if (v9fs_request_cancelled(pdu)) {
err = -EINTR;
any_err |= err = -EINTR;
break;
}
err = s->ops->lstat(&s->ctx, &pathes[name_idx], &stbuf);
err = s->ops->lstat(&s->ctx, &pathes[nwalked], &stbuf);
if (err < 0) {
err = -errno;
any_err |= err = -errno;
break;
}
stbufs[name_idx] = stbuf;
v9fs_path_copy(&dpath, &pathes[name_idx]);
stbufs[nwalked] = stbuf;
v9fs_path_copy(&dpath, &pathes[nwalked]);
}
}
});
/*
* Handle all the rest of this Twalk request on main thread ...
*
* NOTE: -EINTR is an exception where we deviate from the protocol spec
* and simply send a (R)Lerror response instead of bothering to assemble
* a (deducted) Rwalk response; because -EINTR is always the result of a
* Tflush request, so client would no longer wait for a response in this
* case anyway.
*/
if (err < 0) {
if ((err < 0 && !nwalked) || err == -EINTR) {
goto out;
}
err = stat_to_qid(pdu, &fidst, &qid);
if (err < 0) {
any_err |= err = stat_to_qid(pdu, &fidst, &qid);
if (err < 0 && !nwalked) {
goto out;
}
stbuf = fidst;
@ -1890,20 +1897,29 @@ static void coroutine_fn v9fs_walk(void *opaque)
v9fs_path_copy(&dpath, &fidp->path);
v9fs_path_copy(&path, &fidp->path);
for (name_idx = 0; name_idx < nwnames; name_idx++) {
for (name_idx = 0; name_idx < nwalked; name_idx++) {
if (!same_stat_id(&pdu->s->root_st, &stbuf) ||
strcmp("..", wnames[name_idx].data))
{
stbuf = stbufs[name_idx];
err = stat_to_qid(pdu, &stbuf, &qid);
any_err |= err = stat_to_qid(pdu, &stbuf, &qid);
if (err < 0) {
goto out;
break;
}
v9fs_path_copy(&path, &pathes[name_idx]);
v9fs_path_copy(&dpath, &path);
}
memcpy(&qids[name_idx], &qid, sizeof(qid));
}
if (any_err < 0) {
if (!name_idx) {
/* don't send any QIDs, send Rlerror instead */
goto out;
} else {
/* send QIDs (not Rlerror), but fid MUST remain unaffected */
goto send_qids;
}
}
if (fid == newfid) {
if (fidp->fid_type != P9_FID_NONE) {
err = -EINVAL;
@ -1921,8 +1937,9 @@ static void coroutine_fn v9fs_walk(void *opaque)
newfidp->uid = fidp->uid;
v9fs_path_copy(&newfidp->path, &path);
}
err = v9fs_walk_marshal(pdu, nwnames, qids);
trace_v9fs_walk_return(pdu->tag, pdu->id, nwnames, qids);
send_qids:
err = v9fs_walk_marshal(pdu, name_idx, qids);
trace_v9fs_walk_return(pdu->tag, pdu->id, name_idx, qids);
out:
put_fid(pdu, fidp);
if (newfidp) {

View File

@ -371,8 +371,15 @@ static P9Req *v9fs_tattach(QVirtio9P *v9p, uint32_t fid, uint32_t n_uname,
return req;
}
/* type[1] version[4] path[8] */
typedef char v9fs_qid[13];
static inline bool is_same_qid(v9fs_qid a, v9fs_qid b)
{
/* don't compare QID version for checking for file ID equalness */
return a[0] == b[0] && memcmp(&a[5], &b[5], 8) == 0;
}
/* size[4] Rattach tag[2] qid[13] */
static void v9fs_rattach(P9Req *req, v9fs_qid *qid)
{
@ -425,6 +432,79 @@ static void v9fs_rwalk(P9Req *req, uint16_t *nwqid, v9fs_qid **wqid)
v9fs_req_free(req);
}
/* size[4] Tgetattr tag[2] fid[4] request_mask[8] */
static P9Req *v9fs_tgetattr(QVirtio9P *v9p, uint32_t fid, uint64_t request_mask,
uint16_t tag)
{
P9Req *req;
req = v9fs_req_init(v9p, 4 + 8, P9_TGETATTR, tag);
v9fs_uint32_write(req, fid);
v9fs_uint64_write(req, request_mask);
v9fs_req_send(req);
return req;
}
typedef struct v9fs_attr {
uint64_t valid;
v9fs_qid qid;
uint32_t mode;
uint32_t uid;
uint32_t gid;
uint64_t nlink;
uint64_t rdev;
uint64_t size;
uint64_t blksize;
uint64_t blocks;
uint64_t atime_sec;
uint64_t atime_nsec;
uint64_t mtime_sec;
uint64_t mtime_nsec;
uint64_t ctime_sec;
uint64_t ctime_nsec;
uint64_t btime_sec;
uint64_t btime_nsec;
uint64_t gen;
uint64_t data_version;
} v9fs_attr;
#define P9_GETATTR_BASIC 0x000007ffULL /* Mask for fields up to BLOCKS */
/*
* size[4] Rgetattr tag[2] valid[8] qid[13] mode[4] uid[4] gid[4] nlink[8]
* rdev[8] size[8] blksize[8] blocks[8]
* atime_sec[8] atime_nsec[8] mtime_sec[8] mtime_nsec[8]
* ctime_sec[8] ctime_nsec[8] btime_sec[8] btime_nsec[8]
* gen[8] data_version[8]
*/
static void v9fs_rgetattr(P9Req *req, v9fs_attr *attr)
{
v9fs_req_recv(req, P9_RGETATTR);
v9fs_uint64_read(req, &attr->valid);
v9fs_memread(req, &attr->qid, 13);
v9fs_uint32_read(req, &attr->mode);
v9fs_uint32_read(req, &attr->uid);
v9fs_uint32_read(req, &attr->gid);
v9fs_uint64_read(req, &attr->nlink);
v9fs_uint64_read(req, &attr->rdev);
v9fs_uint64_read(req, &attr->size);
v9fs_uint64_read(req, &attr->blksize);
v9fs_uint64_read(req, &attr->blocks);
v9fs_uint64_read(req, &attr->atime_sec);
v9fs_uint64_read(req, &attr->atime_nsec);
v9fs_uint64_read(req, &attr->mtime_sec);
v9fs_uint64_read(req, &attr->mtime_nsec);
v9fs_uint64_read(req, &attr->ctime_sec);
v9fs_uint64_read(req, &attr->ctime_nsec);
v9fs_uint64_read(req, &attr->btime_sec);
v9fs_uint64_read(req, &attr->btime_nsec);
v9fs_uint64_read(req, &attr->gen);
v9fs_uint64_read(req, &attr->data_version);
v9fs_req_free(req);
}
/* size[4] Treaddir tag[2] fid[4] offset[8] count[4] */
static P9Req *v9fs_treaddir(QVirtio9P *v9p, uint32_t fid, uint64_t offset,
uint32_t count, uint16_t tag)
@ -589,8 +669,12 @@ static void do_version(QVirtio9P *v9p)
g_assert_cmpmem(server_version, server_len, version, strlen(version));
}
/* utility function: walk to requested dir and return fid for that dir */
static uint32_t do_walk(QVirtio9P *v9p, const char *path)
/*
* utility function: walk to requested dir and return fid for that dir and
* the QIDs of server response
*/
static uint32_t do_walk_rqids(QVirtio9P *v9p, const char *path, uint16_t *nwqid,
v9fs_qid **wqid)
{
char **wnames;
P9Req *req;
@ -600,26 +684,56 @@ static uint32_t do_walk(QVirtio9P *v9p, const char *path)
req = v9fs_twalk(v9p, 0, fid, nwnames, wnames, 0);
v9fs_req_wait_for_reply(req, NULL);
v9fs_rwalk(req, NULL, NULL);
v9fs_rwalk(req, nwqid, wqid);
split_free(&wnames);
return fid;
}
/* utility function: walk to requested dir and return fid for that dir */
static uint32_t do_walk(QVirtio9P *v9p, const char *path)
{
return do_walk_rqids(v9p, path, NULL, NULL);
}
/* utility function: walk to requested dir and expect passed error response */
static void do_walk_expect_error(QVirtio9P *v9p, const char *path, uint32_t err)
{
char **wnames;
P9Req *req;
uint32_t _err;
const uint32_t fid = genfid();
int nwnames = split(path, "/", &wnames);
req = v9fs_twalk(v9p, 0, fid, nwnames, wnames, 0);
v9fs_req_wait_for_reply(req, NULL);
v9fs_rlerror(req, &_err);
g_assert_cmpint(_err, ==, err);
split_free(&wnames);
}
static void fs_version(void *obj, void *data, QGuestAllocator *t_alloc)
{
alloc = t_alloc;
do_version(obj);
}
static void do_attach(QVirtio9P *v9p)
static void do_attach_rqid(QVirtio9P *v9p, v9fs_qid *qid)
{
P9Req *req;
do_version(v9p);
req = v9fs_tattach(v9p, 0, getuid(), 0);
v9fs_req_wait_for_reply(req, NULL);
v9fs_rattach(req, NULL);
v9fs_rattach(req, qid);
}
static void do_attach(QVirtio9P *v9p)
{
do_attach_rqid(v9p, NULL);
}
static void fs_attach(void *obj, void *data, QGuestAllocator *t_alloc)
@ -974,6 +1088,80 @@ static void fs_walk_no_slash(void *obj, void *data, QGuestAllocator *t_alloc)
g_free(wnames[0]);
}
static void fs_walk_nonexistent(void *obj, void *data, QGuestAllocator *t_alloc)
{
QVirtio9P *v9p = obj;
alloc = t_alloc;
do_attach(v9p);
/*
* The 9p2000 protocol spec says: "If the first element cannot be walked
* for any reason, Rerror is returned."
*/
do_walk_expect_error(v9p, "non-existent", ENOENT);
}
static void fs_walk_2nd_nonexistent(void *obj, void *data,
QGuestAllocator *t_alloc)
{
QVirtio9P *v9p = obj;
alloc = t_alloc;
v9fs_qid root_qid;
uint16_t nwqid;
uint32_t fid, err;
P9Req *req;
g_autofree v9fs_qid *wqid = NULL;
g_autofree char *path = g_strdup_printf(
QTEST_V9FS_SYNTH_WALK_FILE "/non-existent", 0
);
do_attach_rqid(v9p, &root_qid);
fid = do_walk_rqids(v9p, path, &nwqid, &wqid);
/*
* The 9p2000 protocol spec says: "nwqid is therefore either nwname or the
* index of the first elementwise walk that failed."
*/
assert(nwqid == 1);
/* returned QID wqid[0] is file ID of 1st subdir */
g_assert(wqid && wqid[0] && !is_same_qid(root_qid, wqid[0]));
/* expect fid being unaffected by walk above */
req = v9fs_tgetattr(v9p, fid, P9_GETATTR_BASIC, 0);
v9fs_req_wait_for_reply(req, NULL);
v9fs_rlerror(req, &err);
g_assert_cmpint(err, ==, ENOENT);
}
static void fs_walk_none(void *obj, void *data, QGuestAllocator *t_alloc)
{
QVirtio9P *v9p = obj;
alloc = t_alloc;
v9fs_qid root_qid;
g_autofree v9fs_qid *wqid = NULL;
P9Req *req;
struct v9fs_attr attr;
do_version(v9p);
req = v9fs_tattach(v9p, 0, getuid(), 0);
v9fs_req_wait_for_reply(req, NULL);
v9fs_rattach(req, &root_qid);
req = v9fs_twalk(v9p, 0, 1, 0, NULL, 0);
v9fs_req_wait_for_reply(req, NULL);
v9fs_rwalk(req, NULL, &wqid);
/* special case: no QID is returned if nwname=0 was sent */
g_assert(wqid == NULL);
req = v9fs_tgetattr(v9p, 1, P9_GETATTR_BASIC, 0);
v9fs_req_wait_for_reply(req, NULL);
v9fs_rgetattr(req, &attr);
g_assert(is_same_qid(root_qid, attr.qid));
}
static void fs_walk_dotdot(void *obj, void *data, QGuestAllocator *t_alloc)
{
QVirtio9P *v9p = obj;
@ -1407,8 +1595,13 @@ static void register_virtio_9p_test(void)
qos_add_test("synth/walk/basic", "virtio-9p", fs_walk, &opts);
qos_add_test("synth/walk/no_slash", "virtio-9p", fs_walk_no_slash,
&opts);
qos_add_test("synth/walk/none", "virtio-9p", fs_walk_none, &opts);
qos_add_test("synth/walk/dotdot_from_root", "virtio-9p",
fs_walk_dotdot, &opts);
qos_add_test("synth/walk/non_existent", "virtio-9p", fs_walk_nonexistent,
&opts);
qos_add_test("synth/walk/2nd_non_existent", "virtio-9p",
fs_walk_2nd_nonexistent, &opts);
qos_add_test("synth/lopen/basic", "virtio-9p", fs_lopen, &opts);
qos_add_test("synth/write/basic", "virtio-9p", fs_write, &opts);
qos_add_test("synth/flush/success", "virtio-9p", fs_flush_success,