From 64ad31f3284060d59850da6d573ddbab542365cf Mon Sep 17 00:00:00 2001 From: piaojun Date: Tue, 10 Jul 2018 14:56:26 +0800 Subject: net/9p/client.c: add missing '\n' at the end of p9_debug() In p9_client_getattr_dotl(), we should add '\n' at the end of printing log. Link: http://lkml.kernel.org/r/5B44589A.50302@huawei.com Signed-off-by: Jun Piao Reviewed-by: Yiwen Jiang Cc: Eric Van Hensbergen Cc: Ron Minnich Cc: Latchesar Ionkov Signed-off-by: Andrew Morton Signed-off-by: Dominique Martinet --- net/9p/client.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'net/9p') diff --git a/net/9p/client.c b/net/9p/client.c index 5c1343195292..8bc8b3e91532 100644 --- a/net/9p/client.c +++ b/net/9p/client.c @@ -1790,7 +1790,7 @@ struct p9_stat_dotl *p9_client_getattr_dotl(struct p9_fid *fid, "<<< st_mtime_sec=%lld st_mtime_nsec=%lld\n" "<<< st_ctime_sec=%lld st_ctime_nsec=%lld\n" "<<< st_btime_sec=%lld st_btime_nsec=%lld\n" - "<<< st_gen=%lld st_data_version=%lld", + "<<< st_gen=%lld st_data_version=%lld\n", ret->st_result_mask, ret->qid.type, ret->qid.path, ret->qid.version, ret->st_mode, ret->st_nlink, from_kuid(&init_user_ns, ret->st_uid), -- cgit v1.2.3 From b87d1d26521ef9f2e42bb182184d1427b4e07b26 Mon Sep 17 00:00:00 2001 From: piaojun Date: Wed, 11 Jul 2018 08:43:49 +0800 Subject: 9p/net/protocol.c: return -ENOMEM when kmalloc() failed We should return -ENOMEM to upper user when kmalloc failed to indicate accurate errno. Link: http://lkml.kernel.org/r/5B4552C5.60000@huawei.com Signed-off-by: Jun Piao Reviewed-by: Yiwen Jiang Reviewed-by: Andrew Morton Cc: Eric Van Hensbergen Cc: Ron Minnich Cc: Latchesar Ionkov Signed-off-by: Andrew Morton Signed-off-by: Dominique Martinet --- net/9p/protocol.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'net/9p') diff --git a/net/9p/protocol.c b/net/9p/protocol.c index 931ea00c4fed..4a1e1dd30b52 100644 --- a/net/9p/protocol.c +++ b/net/9p/protocol.c @@ -156,7 +156,7 @@ p9pdu_vreadf(struct p9_fcall *pdu, int proto_version, const char *fmt, *sptr = kmalloc(len + 1, GFP_NOFS); if (*sptr == NULL) { - errcode = -EFAULT; + errcode = -ENOMEM; break; } if (pdu_read(pdu, *sptr, len)) { -- cgit v1.2.3 From 92aef4675d5b1b55404e1532379e343bed0e5cf2 Mon Sep 17 00:00:00 2001 From: Jean-Philippe Brucker Date: Tue, 17 Jul 2018 19:14:45 -0700 Subject: net/9p: fix error path of p9_virtio_probe Currently when virtio_find_single_vq fails, we go through del_vqs which throws a warning (Trying to free already-free IRQ). Skip del_vqs if vq allocation failed. Link: http://lkml.kernel.org/r/20180524101021.49880-1-jean-philippe.brucker@arm.com Signed-off-by: Jean-Philippe Brucker Reviewed-by: Greg Kurz Cc: Eric Van Hensbergen Cc: Ron Minnich Cc: Latchesar Ionkov Signed-off-by: Andrew Morton Signed-off-by: Dominique Martinet --- net/9p/trans_virtio.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) (limited to 'net/9p') diff --git a/net/9p/trans_virtio.c b/net/9p/trans_virtio.c index 05006cbb3361..eaacce086427 100644 --- a/net/9p/trans_virtio.c +++ b/net/9p/trans_virtio.c @@ -563,7 +563,7 @@ static int p9_virtio_probe(struct virtio_device *vdev) chan->vq = virtio_find_single_vq(vdev, req_done, "requests"); if (IS_ERR(chan->vq)) { err = PTR_ERR(chan->vq); - goto out_free_vq; + goto out_free_chan; } chan->vq->vdev->priv = chan; spin_lock_init(&chan->lock); @@ -616,6 +616,7 @@ out_free_tag: kfree(tag); out_free_vq: vdev->config->del_vqs(vdev); +out_free_chan: kfree(chan); fail: return err; -- cgit v1.2.3 From 7913690dcc5e18e235769fd87c34143072f5dbea Mon Sep 17 00:00:00 2001 From: Tomas Bortoli Date: Tue, 10 Jul 2018 00:29:43 +0200 Subject: net/9p/client.c: version pointer uninitialized The p9_client_version() does not initialize the version pointer. If the call to p9pdu_readf() returns an error and version has not been allocated in p9pdu_readf(), then the program will jump to the "error" label and will try to free the version pointer. If version is not initialized, free() will be called with uninitialized, garbage data and will provoke a crash. Link: http://lkml.kernel.org/r/20180709222943.19503-1-tomasbortoli@gmail.com Signed-off-by: Tomas Bortoli Reported-by: syzbot+65c6b72f284a39d416b4@syzkaller.appspotmail.com Reviewed-by: Jun Piao Reviewed-by: Yiwen Jiang Cc: Eric Van Hensbergen Cc: Ron Minnich Cc: Latchesar Ionkov Signed-off-by: Andrew Morton Cc: stable@vger.kernel.org Signed-off-by: Dominique Martinet --- net/9p/client.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'net/9p') diff --git a/net/9p/client.c b/net/9p/client.c index 8bc8b3e91532..dd461c718cf9 100644 --- a/net/9p/client.c +++ b/net/9p/client.c @@ -958,7 +958,7 @@ static int p9_client_version(struct p9_client *c) { int err = 0; struct p9_req_t *req; - char *version; + char *version = NULL; int msize; p9_debug(P9_DEBUG_9P, ">>> TVERSION msize %d protocol %d\n", -- cgit v1.2.3 From 2d58f63f72f28ba297a9ae344a5b5f0cf75bcd94 Mon Sep 17 00:00:00 2001 From: Matthew Wilcox Date: Wed, 11 Jul 2018 14:02:20 -0700 Subject: 9p: Fix comment on smp_wmb The previous comment misled me into thinking the barrier wasn't needed at all. Link: http://lkml.kernel.org/r/20180711210225.19730-2-willy@infradead.org Signed-off-by: Matthew Wilcox Reviewed-by: Greg Kurz Cc: Eric Van Hensbergen Cc: Ron Minnich Cc: Latchesar Ionkov Signed-off-by: Dominique Martinet --- net/9p/client.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'net/9p') diff --git a/net/9p/client.c b/net/9p/client.c index dd461c718cf9..cae1fb43bd16 100644 --- a/net/9p/client.c +++ b/net/9p/client.c @@ -448,7 +448,7 @@ void p9_client_cb(struct p9_client *c, struct p9_req_t *req, int status) /* * This barrier is needed to make sure any change made to req before - * the other thread wakes up will indeed be seen by the waiting side. + * the status change is visible to another thread */ smp_wmb(); req->status = status; -- cgit v1.2.3 From b5303be2bee3c8b29de3f7f4ea8ae00c4e816760 Mon Sep 17 00:00:00 2001 From: Matthew Wilcox Date: Wed, 11 Jul 2018 14:02:21 -0700 Subject: 9p: Change p9_fid_create calling convention Return NULL instead of ERR_PTR when we can't allocate a FID. The ENOSPC return value was getting all the way back to userspace, and that's confusing for a userspace program which isn't expecting read() to tell it there's no space left on the filesystem. The best error we can return to indicate a temporary failure caused by lack of client resources is ENOMEM. Maybe it would be better to sleep until a FID is available, but that's not a change I'm comfortable making. Link: http://lkml.kernel.org/r/20180711210225.19730-3-willy@infradead.org Signed-off-by: Matthew Wilcox Reviewed-by: Jun Piao Reviewed-by: Greg Kurz Reviewed-by: Yiwen Jiang Cc: Eric Van Hensbergen Cc: Ron Minnich Cc: Latchesar Ionkov Signed-off-by: Dominique Martinet --- net/9p/client.c | 23 +++++++++-------------- 1 file changed, 9 insertions(+), 14 deletions(-) (limited to 'net/9p') diff --git a/net/9p/client.c b/net/9p/client.c index cae1fb43bd16..7c317d39bf62 100644 --- a/net/9p/client.c +++ b/net/9p/client.c @@ -914,13 +914,11 @@ static struct p9_fid *p9_fid_create(struct p9_client *clnt) p9_debug(P9_DEBUG_FID, "clnt %p\n", clnt); fid = kmalloc(sizeof(struct p9_fid), GFP_KERNEL); if (!fid) - return ERR_PTR(-ENOMEM); + return NULL; ret = p9_idpool_get(clnt->fidpool); - if (ret < 0) { - ret = -ENOSPC; + if (ret < 0) goto error; - } fid->fid = ret; memset(&fid->qid, 0, sizeof(struct p9_qid)); @@ -936,7 +934,7 @@ static struct p9_fid *p9_fid_create(struct p9_client *clnt) error: kfree(fid); - return ERR_PTR(ret); + return NULL; } static void p9_fid_destroy(struct p9_fid *fid) @@ -1138,9 +1136,8 @@ struct p9_fid *p9_client_attach(struct p9_client *clnt, struct p9_fid *afid, p9_debug(P9_DEBUG_9P, ">>> TATTACH afid %d uname %s aname %s\n", afid ? afid->fid : -1, uname, aname); fid = p9_fid_create(clnt); - if (IS_ERR(fid)) { - err = PTR_ERR(fid); - fid = NULL; + if (!fid) { + err = -ENOMEM; goto error; } fid->uid = n_uname; @@ -1189,9 +1186,8 @@ struct p9_fid *p9_client_walk(struct p9_fid *oldfid, uint16_t nwname, clnt = oldfid->clnt; if (clone) { fid = p9_fid_create(clnt); - if (IS_ERR(fid)) { - err = PTR_ERR(fid); - fid = NULL; + if (!fid) { + err = -ENOMEM; goto error; } @@ -2019,9 +2015,8 @@ struct p9_fid *p9_client_xattrwalk(struct p9_fid *file_fid, err = 0; clnt = file_fid->clnt; attr_fid = p9_fid_create(clnt); - if (IS_ERR(attr_fid)) { - err = PTR_ERR(attr_fid); - attr_fid = NULL; + if (!attr_fid) { + err = -ENOMEM; goto error; } p9_debug(P9_DEBUG_9P, -- cgit v1.2.3 From f28cdf0430fc92acaa718e15598bdad6cb236a4d Mon Sep 17 00:00:00 2001 From: Matthew Wilcox Date: Wed, 11 Jul 2018 14:02:22 -0700 Subject: 9p: Replace the fidlist with an IDR The p9_idpool being used to allocate the IDs uses an IDR to allocate the IDs ... which we then keep in a doubly-linked list, rather than in the IDR which allocated them. We can use an IDR directly which saves two pointers per p9_fid, and a tiny memory allocation per p9_client. Link: http://lkml.kernel.org/r/20180711210225.19730-4-willy@infradead.org Signed-off-by: Matthew Wilcox Cc: Eric Van Hensbergen Cc: Ron Minnich Cc: Latchesar Ionkov Signed-off-by: Dominique Martinet --- include/net/9p/client.h | 9 +++------ net/9p/client.c | 44 ++++++++++++++++---------------------------- 2 files changed, 19 insertions(+), 34 deletions(-) (limited to 'net/9p') diff --git a/include/net/9p/client.h b/include/net/9p/client.h index 7af9d769b97d..e405729cd1c7 100644 --- a/include/net/9p/client.h +++ b/include/net/9p/client.h @@ -27,6 +27,7 @@ #define NET_9P_CLIENT_H #include +#include /* Number of requests per row */ #define P9_ROW_MAXTAG 255 @@ -128,8 +129,7 @@ struct p9_req_t { * @proto_version: 9P protocol version to use * @trans_mod: module API instantiated with this client * @trans: tranport instance state and API - * @fidpool: fid handle accounting for session - * @fidlist: List of active fid handles + * @fids: All active FID handles * @tagpool - transaction id accounting for session * @reqs - 2D array of requests * @max_tag - current maximum tag id allocated @@ -169,8 +169,7 @@ struct p9_client { } tcp; } trans_opts; - struct p9_idpool *fidpool; - struct list_head fidlist; + struct idr fids; struct p9_idpool *tagpool; struct p9_req_t *reqs[P9_ROW_MAXTAG]; @@ -188,7 +187,6 @@ struct p9_client { * @iounit: the server reported maximum transaction size for this file * @uid: the numeric uid of the local user who owns this handle * @rdir: readdir accounting structure (allocated on demand) - * @flist: per-client-instance fid tracking * @dlist: per-dentry fid tracking * * TODO: This needs lots of explanation. @@ -204,7 +202,6 @@ struct p9_fid { void *rdir; - struct list_head flist; struct hlist_node dlist; /* list of all fids attached to a dentry */ }; diff --git a/net/9p/client.c b/net/9p/client.c index 7c317d39bf62..cd33cf636c47 100644 --- a/net/9p/client.c +++ b/net/9p/client.c @@ -909,30 +909,29 @@ static struct p9_fid *p9_fid_create(struct p9_client *clnt) { int ret; struct p9_fid *fid; - unsigned long flags; p9_debug(P9_DEBUG_FID, "clnt %p\n", clnt); fid = kmalloc(sizeof(struct p9_fid), GFP_KERNEL); if (!fid) return NULL; - ret = p9_idpool_get(clnt->fidpool); - if (ret < 0) - goto error; - fid->fid = ret; - memset(&fid->qid, 0, sizeof(struct p9_qid)); fid->mode = -1; fid->uid = current_fsuid(); fid->clnt = clnt; fid->rdir = NULL; - spin_lock_irqsave(&clnt->lock, flags); - list_add(&fid->flist, &clnt->fidlist); - spin_unlock_irqrestore(&clnt->lock, flags); + fid->fid = 0; - return fid; + idr_preload(GFP_KERNEL); + spin_lock_irq(&clnt->lock); + ret = idr_alloc_u32(&clnt->fids, fid, &fid->fid, P9_NOFID - 1, + GFP_NOWAIT); + spin_unlock_irq(&clnt->lock); + idr_preload_end(); + + if (!ret) + return fid; -error: kfree(fid); return NULL; } @@ -944,9 +943,8 @@ static void p9_fid_destroy(struct p9_fid *fid) p9_debug(P9_DEBUG_FID, "fid %d\n", fid->fid); clnt = fid->clnt; - p9_idpool_put(fid->fid, clnt->fidpool); spin_lock_irqsave(&clnt->lock, flags); - list_del(&fid->flist); + idr_remove(&clnt->fids, fid->fid); spin_unlock_irqrestore(&clnt->lock, flags); kfree(fid->rdir); kfree(fid); @@ -1029,7 +1027,7 @@ struct p9_client *p9_client_create(const char *dev_name, char *options) memcpy(clnt->name, client_id, strlen(client_id) + 1); spin_lock_init(&clnt->lock); - INIT_LIST_HEAD(&clnt->fidlist); + idr_init(&clnt->fids); err = p9_tag_init(clnt); if (err < 0) @@ -1049,18 +1047,12 @@ struct p9_client *p9_client_create(const char *dev_name, char *options) goto destroy_tagpool; } - clnt->fidpool = p9_idpool_create(); - if (IS_ERR(clnt->fidpool)) { - err = PTR_ERR(clnt->fidpool); - goto put_trans; - } - p9_debug(P9_DEBUG_MUX, "clnt %p trans %p msize %d protocol %d\n", clnt, clnt->trans_mod, clnt->msize, clnt->proto_version); err = clnt->trans_mod->create(clnt, dev_name, options); if (err) - goto destroy_fidpool; + goto put_trans; if (clnt->msize > clnt->trans_mod->maxsize) clnt->msize = clnt->trans_mod->maxsize; @@ -1073,8 +1065,6 @@ struct p9_client *p9_client_create(const char *dev_name, char *options) close_trans: clnt->trans_mod->close(clnt); -destroy_fidpool: - p9_idpool_destroy(clnt->fidpool); put_trans: v9fs_put_trans(clnt->trans_mod); destroy_tagpool: @@ -1087,7 +1077,8 @@ EXPORT_SYMBOL(p9_client_create); void p9_client_destroy(struct p9_client *clnt) { - struct p9_fid *fid, *fidptr; + struct p9_fid *fid; + int id; p9_debug(P9_DEBUG_MUX, "clnt %p\n", clnt); @@ -1096,14 +1087,11 @@ void p9_client_destroy(struct p9_client *clnt) v9fs_put_trans(clnt->trans_mod); - list_for_each_entry_safe(fid, fidptr, &clnt->fidlist, flist) { + idr_for_each_entry(&clnt->fids, fid, id) { pr_info("Found fid %d not clunked\n", fid->fid); p9_fid_destroy(fid); } - if (clnt->fidpool) - p9_idpool_destroy(clnt->fidpool); - p9_tag_cleanup(clnt); kfree(clnt); -- cgit v1.2.3 From 2557d0c57c0c11af915d0d4d97402527958c0c01 Mon Sep 17 00:00:00 2001 From: Matthew Wilcox Date: Wed, 11 Jul 2018 14:02:23 -0700 Subject: 9p: Embed wait_queue_head into p9_req_t On a 64-bit system, the wait_queue_head_t is 24 bytes while the pointer to it is 8 bytes. Growing the p9_req_t by 16 bytes is better than performing a 24-byte memory allocation. Link: http://lkml.kernel.org/r/20180711210225.19730-5-willy@infradead.org Signed-off-by: Matthew Wilcox Reviewed-by: Greg Kurz Cc: Eric Van Hensbergen Cc: Ron Minnich Cc: Latchesar Ionkov Signed-off-by: Dominique Martinet --- include/net/9p/client.h | 2 +- net/9p/client.c | 19 +++++-------------- net/9p/trans_virtio.c | 2 +- 3 files changed, 7 insertions(+), 16 deletions(-) (limited to 'net/9p') diff --git a/include/net/9p/client.h b/include/net/9p/client.h index e405729cd1c7..0fa0fbab33b0 100644 --- a/include/net/9p/client.h +++ b/include/net/9p/client.h @@ -113,7 +113,7 @@ enum p9_req_status_t { struct p9_req_t { int status; int t_err; - wait_queue_head_t *wq; + wait_queue_head_t wq; struct p9_fcall *tc; struct p9_fcall *rc; void *aux; diff --git a/net/9p/client.c b/net/9p/client.c index cd33cf636c47..33717b1b84d8 100644 --- a/net/9p/client.c +++ b/net/9p/client.c @@ -283,8 +283,9 @@ p9_tag_alloc(struct p9_client *c, u16 tag, unsigned int max_size) return ERR_PTR(-ENOMEM); } for (col = 0; col < P9_ROW_MAXTAG; col++) { - c->reqs[row][col].status = REQ_STATUS_IDLE; - c->reqs[row][col].tc = NULL; + req = &c->reqs[row][col]; + req->status = REQ_STATUS_IDLE; + init_waitqueue_head(&req->wq); } c->max_tag += P9_ROW_MAXTAG; } @@ -294,13 +295,6 @@ p9_tag_alloc(struct p9_client *c, u16 tag, unsigned int max_size) col = tag % P9_ROW_MAXTAG; req = &c->reqs[row][col]; - if (!req->wq) { - req->wq = kmalloc(sizeof(wait_queue_head_t), GFP_NOFS); - if (!req->wq) - goto grow_failed; - init_waitqueue_head(req->wq); - } - if (!req->tc) req->tc = p9_fcall_alloc(alloc_msize); if (!req->rc) @@ -320,9 +314,7 @@ grow_failed: pr_err("Couldn't grow tag array\n"); kfree(req->tc); kfree(req->rc); - kfree(req->wq); req->tc = req->rc = NULL; - req->wq = NULL; return ERR_PTR(-ENOMEM); } @@ -410,7 +402,6 @@ static void p9_tag_cleanup(struct p9_client *c) /* free requests associated with tags */ for (row = 0; row < (c->max_tag/P9_ROW_MAXTAG); row++) { for (col = 0; col < P9_ROW_MAXTAG; col++) { - kfree(c->reqs[row][col].wq); kfree(c->reqs[row][col].tc); kfree(c->reqs[row][col].rc); } @@ -453,7 +444,7 @@ void p9_client_cb(struct p9_client *c, struct p9_req_t *req, int status) smp_wmb(); req->status = status; - wake_up(req->wq); + wake_up(&req->wq); p9_debug(P9_DEBUG_MUX, "wakeup: %d\n", req->tc->tag); } EXPORT_SYMBOL(p9_client_cb); @@ -774,7 +765,7 @@ p9_client_rpc(struct p9_client *c, int8_t type, const char *fmt, ...) } again: /* Wait for the response */ - err = wait_event_killable(*req->wq, req->status >= REQ_STATUS_RCVD); + err = wait_event_killable(req->wq, req->status >= REQ_STATUS_RCVD); /* * Make sure our req is coherent with regard to updates in other diff --git a/net/9p/trans_virtio.c b/net/9p/trans_virtio.c index eaacce086427..3f69c428ddf9 100644 --- a/net/9p/trans_virtio.c +++ b/net/9p/trans_virtio.c @@ -490,7 +490,7 @@ req_retry_pinned: virtqueue_kick(chan->vq); spin_unlock_irqrestore(&chan->lock, flags); p9_debug(P9_DEBUG_TRANS, "virtio request kicked\n"); - err = wait_event_killable(*req->wq, req->status >= REQ_STATUS_RCVD); + err = wait_event_killable(req->wq, req->status >= REQ_STATUS_RCVD); /* * Non kernel buffers are pinned, unpin them */ -- cgit v1.2.3 From d28c756caee6e414d9ba367d0b92da24145af2a8 Mon Sep 17 00:00:00 2001 From: Chirantan Ekbote Date: Mon, 16 Jul 2018 17:35:29 -0700 Subject: 9p/net: Fix zero-copy path in the 9p virtio transport The zero-copy optimization when reading or writing large chunks of data is quite useful. However, the 9p messages created through the zero-copy write path have an incorrect message size: it should be the size of the header + size of the data being written but instead it's just the size of the header. This only works if the server ignores the size field of the message and otherwise breaks the framing of the protocol. Fix this by re-writing the message size field with the correct value. Tested by running `dd if=/dev/zero of=out bs=4k count=1` inside a virtio-9p mount. Link: http://lkml.kernel.org/r/20180717003529.114368-1-chirantan@chromium.org Signed-off-by: Chirantan Ekbote Reviewed-by: Greg Kurz Tested-by: Greg Kurz Cc: Dylan Reid Cc: Guenter Roeck Cc: stable@vger.kernel.org Signed-off-by: Dominique Martinet --- net/9p/trans_virtio.c | 7 +++++++ 1 file changed, 7 insertions(+) (limited to 'net/9p') diff --git a/net/9p/trans_virtio.c b/net/9p/trans_virtio.c index 3f69c428ddf9..bf61ca20e6a5 100644 --- a/net/9p/trans_virtio.c +++ b/net/9p/trans_virtio.c @@ -406,6 +406,7 @@ p9_virtio_zc_request(struct p9_client *client, struct p9_req_t *req, p9_debug(P9_DEBUG_TRANS, "virtio request\n"); if (uodata) { + __le32 sz; int n = p9_get_mapped_pages(chan, &out_pages, uodata, outlen, &offs, &need_drop); if (n < 0) @@ -416,6 +417,12 @@ p9_virtio_zc_request(struct p9_client *client, struct p9_req_t *req, memcpy(&req->tc->sdata[req->tc->size - 4], &v, 4); outlen = n; } + /* The size field of the message must include the length of the + * header and the length of the data. We didn't actually know + * the length of the data until this point so add it in now. + */ + sz = cpu_to_le32(req->tc->size + outlen); + memcpy(&req->tc->sdata[0], &sz, sizeof(sz)); } else if (uidata) { int n = p9_get_mapped_pages(chan, &in_pages, uidata, inlen, &offs, &need_drop); -- cgit v1.2.3 From c7ebbae7cf9c50253a978f25d72d16e012bd46f1 Mon Sep 17 00:00:00 2001 From: piaojun Date: Wed, 18 Jul 2018 11:45:29 +0800 Subject: net/9p/trans_virtio.c: fix some spell mistakes in comments Fix spelling mistake in comments of p9_virtio_zc_request(). Link: http://lkml.kernel.org/r/5B4EB7D9.9010108@huawei.com Signed-off-by: Jun Piao Cc: Eric Van Hensbergen Cc: Ron Minnich Cc: Latchesar Ionkov Cc: Andrew Morton Signed-off-by: Dominique Martinet --- net/9p/trans_virtio.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) (limited to 'net/9p') diff --git a/net/9p/trans_virtio.c b/net/9p/trans_virtio.c index bf61ca20e6a5..c5464ec311fe 100644 --- a/net/9p/trans_virtio.c +++ b/net/9p/trans_virtio.c @@ -382,8 +382,8 @@ static int p9_get_mapped_pages(struct virtio_chan *chan, * p9_virtio_zc_request - issue a zero copy request * @client: client instance issuing the request * @req: request to be issued - * @uidata: user bffer that should be ued for zero copy read - * @uodata: user buffer that shoud be user for zero copy write + * @uidata: user buffer that should be used for zero copy read + * @uodata: user buffer that should be used for zero copy write * @inlen: read buffer size * @outlen: write buffer size * @in_hdr_len: reader header size, This is the size of response protocol data -- cgit v1.2.3 From 31934da810365f603dec5a67e690e00cf900fc73 Mon Sep 17 00:00:00 2001 From: jiangyiwen Date: Thu, 19 Jul 2018 15:17:00 +0800 Subject: net/9p/virtio: Fix hard lockup in req_done When client has multiple threads that issue io requests all the time, and the server has a very good performance, it may cause cpu is running in the irq context for a long time because it can check virtqueue has buf in the *while* loop. So we should keep chan->lock in the whole loop. [ Dominique: reworded subject line ] Link: http://lkml.kernel.org/r/5B503AEC.5080404@huawei.com Signed-off-by: Yiwen Jiang To: Andrew Morton To: Eric Van Hensbergen To: Ron Minnich To: Latchesar Ionkov Signed-off-by: Dominique Martinet --- net/9p/trans_virtio.c | 21 +++++++++++---------- 1 file changed, 11 insertions(+), 10 deletions(-) (limited to 'net/9p') diff --git a/net/9p/trans_virtio.c b/net/9p/trans_virtio.c index c5464ec311fe..d422bfc81eca 100644 --- a/net/9p/trans_virtio.c +++ b/net/9p/trans_virtio.c @@ -144,24 +144,25 @@ static void req_done(struct virtqueue *vq) struct virtio_chan *chan = vq->vdev->priv; unsigned int len; struct p9_req_t *req; + bool need_wakeup = false; unsigned long flags; p9_debug(P9_DEBUG_TRANS, ": request done\n"); - while (1) { - spin_lock_irqsave(&chan->lock, flags); - req = virtqueue_get_buf(chan->vq, &len); - if (req == NULL) { - spin_unlock_irqrestore(&chan->lock, flags); - break; + spin_lock_irqsave(&chan->lock, flags); + while ((req = virtqueue_get_buf(chan->vq, &len)) != NULL) { + if (!chan->ring_bufs_avail) { + chan->ring_bufs_avail = 1; + need_wakeup = true; } - chan->ring_bufs_avail = 1; - spin_unlock_irqrestore(&chan->lock, flags); - /* Wakeup if anyone waiting for VirtIO ring space. */ - wake_up(chan->vc_wq); + if (len) p9_client_cb(chan->client, req, REQ_STATUS_RCVD); } + spin_unlock_irqrestore(&chan->lock, flags); + /* Wakeup if anyone waiting for VirtIO ring space. */ + if (need_wakeup) + wake_up(chan->vc_wq); } /** -- cgit v1.2.3 From 430ac66eb4c5b5c4eb846b78ebf65747510b30f1 Mon Sep 17 00:00:00 2001 From: Tomas Bortoli Date: Fri, 20 Jul 2018 11:27:30 +0200 Subject: net/9p/trans_fd.c: fix race-condition by flushing workqueue before the kfree() The patch adds the flush in p9_mux_poll_stop() as it the function used by p9_conn_destroy(), in turn called by p9_fd_close() to stop the async polling associated with the data regarding the connection. Link: http://lkml.kernel.org/r/20180720092730.27104-1-tomasbortoli@gmail.com Signed-off-by: Tomas Bortoli Reported-by: syzbot+39749ed7d9ef6dfb23f6@syzkaller.appspotmail.com To: Eric Van Hensbergen To: Ron Minnich To: Latchesar Ionkov Cc: Yiwen Jiang Cc: stable@vger.kernel.org Signed-off-by: Dominique Martinet --- net/9p/trans_fd.c | 2 ++ 1 file changed, 2 insertions(+) (limited to 'net/9p') diff --git a/net/9p/trans_fd.c b/net/9p/trans_fd.c index 588bf88c3305..b161184f77f6 100644 --- a/net/9p/trans_fd.c +++ b/net/9p/trans_fd.c @@ -185,6 +185,8 @@ static void p9_mux_poll_stop(struct p9_conn *m) spin_lock_irqsave(&p9_poll_lock, flags); list_del_init(&m->poll_pending_link); spin_unlock_irqrestore(&p9_poll_lock, flags); + + flush_work(&p9_poll_work); } /** -- cgit v1.2.3 From 9f476d7c540cb57556d3cc7e78704e6cd5100f5f Mon Sep 17 00:00:00 2001 From: Tomas Bortoli Date: Mon, 23 Jul 2018 20:42:53 +0200 Subject: net/9p/trans_fd.c: fix race by holding the lock It may be possible to run p9_fd_cancel() with a deleted req->req_list and incur in a double del. To fix hold the client->lock while changing the status, so the other threads will be synchronized. Link: http://lkml.kernel.org/r/20180723184253.6682-1-tomasbortoli@gmail.com Signed-off-by: Tomas Bortoli Reported-by: syzbot+735d926e9d1317c3310c@syzkaller.appspotmail.com To: Eric Van Hensbergen To: Ron Minnich To: Latchesar Ionkov Cc: Yiwen Jiang Cc: David S. Miller Signed-off-by: Dominique Martinet --- net/9p/trans_fd.c | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) (limited to 'net/9p') diff --git a/net/9p/trans_fd.c b/net/9p/trans_fd.c index b161184f77f6..cce41b20a709 100644 --- a/net/9p/trans_fd.c +++ b/net/9p/trans_fd.c @@ -199,15 +199,14 @@ static void p9_mux_poll_stop(struct p9_conn *m) static void p9_conn_cancel(struct p9_conn *m, int err) { struct p9_req_t *req, *rtmp; - unsigned long flags; LIST_HEAD(cancel_list); p9_debug(P9_DEBUG_ERROR, "mux %p err %d\n", m, err); - spin_lock_irqsave(&m->client->lock, flags); + spin_lock(&m->client->lock); if (m->err) { - spin_unlock_irqrestore(&m->client->lock, flags); + spin_unlock(&m->client->lock); return; } @@ -219,7 +218,6 @@ static void p9_conn_cancel(struct p9_conn *m, int err) list_for_each_entry_safe(req, rtmp, &m->unsent_req_list, req_list) { list_move(&req->req_list, &cancel_list); } - spin_unlock_irqrestore(&m->client->lock, flags); list_for_each_entry_safe(req, rtmp, &cancel_list, req_list) { p9_debug(P9_DEBUG_ERROR, "call back req %p\n", req); @@ -228,6 +226,7 @@ static void p9_conn_cancel(struct p9_conn *m, int err) req->t_err = err; p9_client_cb(m->client, req, REQ_STATUS_ERROR); } + spin_unlock(&m->client->lock); } static __poll_t @@ -375,8 +374,9 @@ static void p9_read_work(struct work_struct *work) if (m->req->status != REQ_STATUS_ERROR) status = REQ_STATUS_RCVD; list_del(&m->req->req_list); - spin_unlock(&m->client->lock); + /* update req->status while holding client->lock */ p9_client_cb(m->client, m->req, status); + spin_unlock(&m->client->lock); m->rc.sdata = NULL; m->rc.offset = 0; m->rc.capacity = 0; -- cgit v1.2.3 From f984579a01d85166ee7380204a96d978a67687a1 Mon Sep 17 00:00:00 2001 From: Tomas Bortoli Date: Mon, 23 Jul 2018 17:44:04 +0200 Subject: 9p: validate PDU length This commit adds length check for the PDU size. The size contained in the header has to match the actual size, except for TCP (trans_fd.c) where actual length is not known ahead and the header's length will be checked only against the validity range. Link: http://lkml.kernel.org/r/20180723154404.2406-1-tomasbortoli@gmail.com Signed-off-by: Tomas Bortoli Reported-by: syzbot+65c6b72f284a39d416b4@syzkaller.appspotmail.com To: Eric Van Hensbergen To: Ron Minnich To: Latchesar Ionkov Cc: David S. Miller Signed-off-by: Dominique Martinet --- net/9p/client.c | 25 ++++++++++++++++--------- net/9p/trans_fd.c | 5 ++++- net/9p/trans_rdma.c | 1 + net/9p/trans_virtio.c | 4 +++- 4 files changed, 24 insertions(+), 11 deletions(-) (limited to 'net/9p') diff --git a/net/9p/client.c b/net/9p/client.c index 33717b1b84d8..20088aa06343 100644 --- a/net/9p/client.c +++ b/net/9p/client.c @@ -469,20 +469,11 @@ p9_parse_header(struct p9_fcall *pdu, int32_t *size, int8_t *type, int16_t *tag, int err; pdu->offset = 0; - if (pdu->size == 0) - pdu->size = 7; err = p9pdu_readf(pdu, 0, "dbw", &r_size, &r_type, &r_tag); if (err) goto rewind_and_exit; - pdu->size = r_size; - pdu->id = r_type; - pdu->tag = r_tag; - - p9_debug(P9_DEBUG_9P, "<<< size=%d type: %d tag: %d\n", - pdu->size, pdu->id, pdu->tag); - if (type) *type = r_type; if (tag) @@ -490,6 +481,16 @@ p9_parse_header(struct p9_fcall *pdu, int32_t *size, int8_t *type, int16_t *tag, if (size) *size = r_size; + if (pdu->size != r_size || r_size < 7) { + err = -EINVAL; + goto rewind_and_exit; + } + + pdu->id = r_type; + pdu->tag = r_tag; + + p9_debug(P9_DEBUG_9P, "<<< size=%d type: %d tag: %d\n", + pdu->size, pdu->id, pdu->tag); rewind_and_exit: if (rewind) @@ -516,6 +517,12 @@ static int p9_check_errors(struct p9_client *c, struct p9_req_t *req) int ecode; err = p9_parse_header(req->rc, NULL, &type, NULL, 0); + if (req->rc->size >= c->msize) { + p9_debug(P9_DEBUG_ERROR, + "requested packet size too big: %d\n", + req->rc->size); + return -EIO; + } /* * dump the response from server * This should be after check errors which poplulate pdu_fcall. diff --git a/net/9p/trans_fd.c b/net/9p/trans_fd.c index cce41b20a709..964260265b13 100644 --- a/net/9p/trans_fd.c +++ b/net/9p/trans_fd.c @@ -325,7 +325,9 @@ static void p9_read_work(struct work_struct *work) if ((!m->req) && (m->rc.offset == m->rc.capacity)) { p9_debug(P9_DEBUG_TRANS, "got new header\n"); - err = p9_parse_header(&m->rc, NULL, NULL, NULL, 0); + /* Header size */ + m->rc.size = 7; + err = p9_parse_header(&m->rc, &m->rc.size, NULL, NULL, 0); if (err) { p9_debug(P9_DEBUG_ERROR, "error parsing header: %d\n", err); @@ -370,6 +372,7 @@ static void p9_read_work(struct work_struct *work) */ if ((m->req) && (m->rc.offset == m->rc.capacity)) { p9_debug(P9_DEBUG_TRANS, "got new packet\n"); + m->req->rc->size = m->rc.offset; spin_lock(&m->client->lock); if (m->req->status != REQ_STATUS_ERROR) status = REQ_STATUS_RCVD; diff --git a/net/9p/trans_rdma.c b/net/9p/trans_rdma.c index 3d414acb7015..2649b2ebf961 100644 --- a/net/9p/trans_rdma.c +++ b/net/9p/trans_rdma.c @@ -320,6 +320,7 @@ recv_done(struct ib_cq *cq, struct ib_wc *wc) if (wc->status != IB_WC_SUCCESS) goto err_out; + c->rc->size = wc->byte_len; err = p9_parse_header(c->rc, NULL, NULL, &tag, 1); if (err) goto err_out; diff --git a/net/9p/trans_virtio.c b/net/9p/trans_virtio.c index d422bfc81eca..06dcd3cc6a29 100644 --- a/net/9p/trans_virtio.c +++ b/net/9p/trans_virtio.c @@ -156,8 +156,10 @@ static void req_done(struct virtqueue *vq) need_wakeup = true; } - if (len) + if (len) { + req->rc->size = len; p9_client_cb(chan->client, req, REQ_STATUS_RCVD); + } } spin_unlock_irqrestore(&chan->lock, flags); /* Wakeup if anyone waiting for VirtIO ring space. */ -- cgit v1.2.3 From 10aa14527f458e9867cf3d2cc6b8cb0f6704448b Mon Sep 17 00:00:00 2001 From: Tomas Bortoli Date: Fri, 27 Jul 2018 13:05:58 +0200 Subject: 9p: fix multiple NULL-pointer-dereferences Added checks to prevent GPFs from raising. Link: http://lkml.kernel.org/r/20180727110558.5479-1-tomasbortoli@gmail.com Signed-off-by: Tomas Bortoli Reported-by: syzbot+1a262da37d3bead15c39@syzkaller.appspotmail.com Cc: stable@vger.kernel.org Signed-off-by: Dominique Martinet --- net/9p/trans_fd.c | 5 ++++- net/9p/trans_rdma.c | 3 +++ net/9p/trans_virtio.c | 3 +++ net/9p/trans_xen.c | 3 +++ 4 files changed, 13 insertions(+), 1 deletion(-) (limited to 'net/9p') diff --git a/net/9p/trans_fd.c b/net/9p/trans_fd.c index 964260265b13..e2ef3c782c53 100644 --- a/net/9p/trans_fd.c +++ b/net/9p/trans_fd.c @@ -945,7 +945,7 @@ p9_fd_create_tcp(struct p9_client *client, const char *addr, char *args) if (err < 0) return err; - if (valid_ipaddr4(addr) < 0) + if (addr == NULL || valid_ipaddr4(addr) < 0) return -EINVAL; csocket = NULL; @@ -995,6 +995,9 @@ p9_fd_create_unix(struct p9_client *client, const char *addr, char *args) csocket = NULL; + if (addr == NULL) + return -EINVAL; + if (strlen(addr) >= UNIX_PATH_MAX) { pr_err("%s (%d): address too long: %s\n", __func__, task_pid_nr(current), addr); diff --git a/net/9p/trans_rdma.c b/net/9p/trans_rdma.c index 2649b2ebf961..2ab4574183c9 100644 --- a/net/9p/trans_rdma.c +++ b/net/9p/trans_rdma.c @@ -645,6 +645,9 @@ rdma_create_trans(struct p9_client *client, const char *addr, char *args) struct rdma_conn_param conn_param; struct ib_qp_init_attr qp_attr; + if (addr == NULL) + return -EINVAL; + /* Parse the transport specific mount options */ err = parse_opts(args, &opts); if (err < 0) diff --git a/net/9p/trans_virtio.c b/net/9p/trans_virtio.c index 06dcd3cc6a29..8ca356eb66bb 100644 --- a/net/9p/trans_virtio.c +++ b/net/9p/trans_virtio.c @@ -654,6 +654,9 @@ p9_virtio_create(struct p9_client *client, const char *devname, char *args) int ret = -ENOENT; int found = 0; + if (devname == NULL) + return -EINVAL; + mutex_lock(&virtio_9p_lock); list_for_each_entry(chan, &virtio_chan_list, chan_list) { if (!strncmp(devname, chan->tag, chan->tag_len) && diff --git a/net/9p/trans_xen.c b/net/9p/trans_xen.c index 2e2b8bca54f3..c2d54ac76bfd 100644 --- a/net/9p/trans_xen.c +++ b/net/9p/trans_xen.c @@ -94,6 +94,9 @@ static int p9_xen_create(struct p9_client *client, const char *addr, char *args) { struct xen_9pfs_front_priv *priv; + if (addr == NULL) + return -EINVAL; + read_lock(&xen_9pfs_lock); list_for_each_entry(priv, &xen_9pfs_devs, list) { if (!strcmp(priv->tag, addr)) { -- cgit v1.2.3 From c69f297d7f0b46187919d5c2aa36183688ecf3d4 Mon Sep 17 00:00:00 2001 From: Stephen Hemminger Date: Tue, 24 Jul 2018 12:29:10 -0700 Subject: 9p: fix whitespace issues Remove trailing whitespace and blank lines at EOF Link: http://lkml.kernel.org/m/20180724192918.31165-11-sthemmin@microsoft.com Signed-off-by: Stephen Hemminger Signed-off-by: Dominique Martinet --- net/9p/client.c | 4 ++-- net/9p/trans_virtio.c | 2 +- net/9p/util.c | 1 - 3 files changed, 3 insertions(+), 4 deletions(-) (limited to 'net/9p') diff --git a/net/9p/client.c b/net/9p/client.c index 20088aa06343..deae53a7dffc 100644 --- a/net/9p/client.c +++ b/net/9p/client.c @@ -333,7 +333,7 @@ struct p9_req_t *p9_tag_lookup(struct p9_client *c, u16 tag) * buffer to read the data into */ tag++; - if(tag >= c->max_tag) + if (tag >= c->max_tag) return NULL; row = tag / P9_ROW_MAXTAG; @@ -1558,7 +1558,7 @@ p9_client_read(struct p9_fid *fid, u64 offset, struct iov_iter *to, int *err) int count = iov_iter_count(to); int rsize, non_zc = 0; char *dataptr; - + rsize = fid->iounit; if (!rsize || rsize > clnt->msize-P9_IOHDRSZ) rsize = clnt->msize - P9_IOHDRSZ; diff --git a/net/9p/trans_virtio.c b/net/9p/trans_virtio.c index 8ca356eb66bb..6265d1d62749 100644 --- a/net/9p/trans_virtio.c +++ b/net/9p/trans_virtio.c @@ -456,7 +456,7 @@ req_retry_pinned: out += pack_sg_list_p(chan->sg, out, VIRTQUEUE_NUM, out_pages, out_nr_pages, offs, outlen); } - + /* * Take care of in data * For example TREAD have 11. diff --git a/net/9p/util.c b/net/9p/util.c index 59f278e64f58..55ad98277e85 100644 --- a/net/9p/util.c +++ b/net/9p/util.c @@ -138,4 +138,3 @@ int p9_idpool_check(int id, struct p9_idpool *p) return idr_find(&p->pool, id) != NULL; } EXPORT_SYMBOL(p9_idpool_check); - -- cgit v1.2.3 From 23cba9cbde0bba05d772b335fe5f66aa82b9ad19 Mon Sep 17 00:00:00 2001 From: jiangyiwen Date: Fri, 3 Aug 2018 12:11:34 +0800 Subject: 9p/virtio: fix off-by-one error in sg list bounds check Because the value of limit is VIRTQUEUE_NUM, if index is equal to limit, it will cause sg array out of bounds, so correct the judgement of BUG_ON. Link: http://lkml.kernel.org/r/5B63D5F6.6080109@huawei.com Signed-off-by: Yiwen Jiang Reported-By: Dan Carpenter Acked-by: Jun Piao Cc: stable@vger.kernel.org Signed-off-by: Dominique Martinet --- net/9p/trans_virtio.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) (limited to 'net/9p') diff --git a/net/9p/trans_virtio.c b/net/9p/trans_virtio.c index 6265d1d62749..08264bae3f5d 100644 --- a/net/9p/trans_virtio.c +++ b/net/9p/trans_virtio.c @@ -191,7 +191,7 @@ static int pack_sg_list(struct scatterlist *sg, int start, s = rest_of_page(data); if (s > count) s = count; - BUG_ON(index > limit); + BUG_ON(index >= limit); /* Make sure we don't terminate early. */ sg_unmark_end(&sg[index]); sg_set_buf(&sg[index++], data, s); @@ -236,6 +236,7 @@ pack_sg_list_p(struct scatterlist *sg, int start, int limit, s = PAGE_SIZE - data_off; if (s > count) s = count; + BUG_ON(index >= limit); /* Make sure we don't terminate early. */ sg_unmark_end(&sg[index]); sg_set_page(&sg[index++], pdata[i++], s, data_off); -- cgit v1.2.3 From edcd9d977354304cb85aee61c2b96809edce41ed Mon Sep 17 00:00:00 2001 From: piaojun Date: Fri, 3 Aug 2018 17:22:20 +0800 Subject: net/9p/trans_virtio.c: add null terminal for mount tag chan->tag is Non-null terminated which will result in printing messy code when debugging code. So we should add '\0' for tag to make the code more convenient and robust. In addition, I drop char->tag_len to simplify the code. Link: http://lkml.kernel.org/r/5B641ECC.5030401@huawei.com Signed-off-by: Jun Piao Signed-off-by: Dominique Martinet --- net/9p/trans_virtio.c | 17 +++++++---------- 1 file changed, 7 insertions(+), 10 deletions(-) (limited to 'net/9p') diff --git a/net/9p/trans_virtio.c b/net/9p/trans_virtio.c index 08264bae3f5d..7728b0acde09 100644 --- a/net/9p/trans_virtio.c +++ b/net/9p/trans_virtio.c @@ -89,10 +89,8 @@ struct virtio_chan { unsigned long p9_max_pages; /* Scatterlist: can be too big for stack. */ struct scatterlist sg[VIRTQUEUE_NUM]; - - int tag_len; /* - * tag name to identify a mount Non-null terminated + * tag name to identify a mount null terminated */ char *tag; @@ -528,14 +526,15 @@ static ssize_t p9_mount_tag_show(struct device *dev, { struct virtio_chan *chan; struct virtio_device *vdev; + int tag_len; vdev = dev_to_virtio(dev); chan = vdev->priv; + tag_len = strlen(chan->tag); - memcpy(buf, chan->tag, chan->tag_len); - buf[chan->tag_len] = 0; + memcpy(buf, chan->tag, tag_len + 1); - return chan->tag_len + 1; + return tag_len + 1; } static DEVICE_ATTR(mount_tag, 0444, p9_mount_tag_show, NULL); @@ -588,7 +587,7 @@ static int p9_virtio_probe(struct virtio_device *vdev) err = -EINVAL; goto out_free_vq; } - tag = kmalloc(tag_len, GFP_KERNEL); + tag = kzalloc(tag_len + 1, GFP_KERNEL); if (!tag) { err = -ENOMEM; goto out_free_vq; @@ -597,7 +596,6 @@ static int p9_virtio_probe(struct virtio_device *vdev) virtio_cread_bytes(vdev, offsetof(struct virtio_9p_config, tag), tag, tag_len); chan->tag = tag; - chan->tag_len = tag_len; err = sysfs_create_file(&(vdev->dev.kobj), &dev_attr_mount_tag.attr); if (err) { goto out_free_tag; @@ -660,8 +658,7 @@ p9_virtio_create(struct p9_client *client, const char *devname, char *args) mutex_lock(&virtio_9p_lock); list_for_each_entry(chan, &virtio_chan_list, chan_list) { - if (!strncmp(devname, chan->tag, chan->tag_len) && - strlen(devname) == chan->tag_len) { + if (!strcmp(devname, chan->tag)) { if (!chan->inuse) { chan->inuse = true; found = 1; -- cgit v1.2.3