From e8782e2fbcb5c1c8823a733a985a2896a1450334 Mon Sep 17 00:00:00 2001 From: Fabian Frederick Date: Fri, 4 Jul 2014 20:28:44 +0200 Subject: 9p: kerneldoc warning fixes options argument was removed from v9fs_session_info in commit 4b53e4b50077 ("9p: remove unnecessary v9fses->options which duplicates the mount string") iov and nr_segs were removed from v9fs_direct_IO in commit d8d3d94b80aa ("pass iov_iter to ->direct_IO()") Cc: Eric Van Hensbergen Cc: Ron Minnich Cc: Latchesar Ionkov Cc: v9fs-developer@lists.sourceforge.net Signed-off-by: Fabian Frederick Signed-off-by: Dominique Martinet Signed-off-by: Eric Van Hensbergen diff --git a/fs/9p/v9fs.h b/fs/9p/v9fs.h index 099c771..fb9ffcb 100644 --- a/fs/9p/v9fs.h +++ b/fs/9p/v9fs.h @@ -78,7 +78,6 @@ enum p9_cache_modes { * @cache: cache mode of type &p9_cache_modes * @cachetag: the tag of the cache associated with this session * @fscache: session cookie associated with FS-Cache - * @options: copy of options string given by user * @uname: string user name to mount hierarchy as * @aname: mount specifier for remote hierarchy * @maxdata: maximum data to be sent/recvd per protocol message diff --git a/fs/9p/vfs_addr.c b/fs/9p/vfs_addr.c index eb14e05..3672b16 100644 --- a/fs/9p/vfs_addr.c +++ b/fs/9p/vfs_addr.c @@ -243,9 +243,7 @@ static int v9fs_launder_page(struct page *page) * v9fs_direct_IO - 9P address space operation for direct I/O * @rw: direction (read or write) * @iocb: target I/O control block - * @iov: array of vectors that define I/O buffer * @pos: offset in file to begin the operation - * @nr_segs: size of iovec array * * The presence of v9fs_direct_IO() in the address space ops vector * allowes open() O_DIRECT flags which would have failed otherwise. -- cgit v0.10.2 From 9bfc52c1091c871cbc58390874b5c4ebe266bee0 Mon Sep 17 00:00:00 2001 From: Fabian Frederick Date: Thu, 23 Oct 2014 18:19:35 +0200 Subject: 9p: remove unused variable in p9_fd_create() p is initialized but unused. Signed-off-by: Fabian Frederick Signed-off-by: Dominique Martinet Signed-off-by: Eric Van Hensbergen diff --git a/net/9p/trans_fd.c b/net/9p/trans_fd.c index 80d08f6..c73b894 100644 --- a/net/9p/trans_fd.c +++ b/net/9p/trans_fd.c @@ -1013,7 +1013,6 @@ p9_fd_create(struct p9_client *client, const char *addr, char *args) { int err; struct p9_fd_opts opts; - struct p9_trans_fd *p; parse_opts(args, &opts); @@ -1026,7 +1025,6 @@ p9_fd_create(struct p9_client *client, const char *addr, char *args) if (err < 0) return err; - p = (struct p9_trans_fd *) client->trans; p9_conn_create(client); return 0; -- cgit v0.10.2 From ad80492df56b4bd2d4da9990678d87b66af42f54 Mon Sep 17 00:00:00 2001 From: "Kirill A. Shutemov" Date: Mon, 29 Dec 2014 15:00:18 +0200 Subject: 9p: fix error handling in v9fs_file_do_lock p9_client_lock_dotl() doesn't set status if p9_client_rpc() fails. It can lead to 'default:' case in switch below and kernel crashes. Let's bypass the switch if p9_client_lock_dotl() fails. Signed-off-by: Kirill A. Shutemov Signed-off-by: Dominique Martinet Signed-off-by: Eric Van Hensbergen diff --git a/fs/9p/vfs_file.c b/fs/9p/vfs_file.c index b401337..8d29e1e 100644 --- a/fs/9p/vfs_file.c +++ b/fs/9p/vfs_file.c @@ -194,7 +194,7 @@ static int v9fs_file_do_lock(struct file *filp, int cmd, struct file_lock *fl) for (;;) { res = p9_client_lock_dotl(fid, &flock, &status); if (res < 0) - break; + goto out_unlock; if (status != P9_LOCK_BLOCKED) break; @@ -220,6 +220,7 @@ static int v9fs_file_do_lock(struct file *filp, int cmd, struct file_lock *fl) BUG(); } +out_unlock: /* * incase server returned error for lock request, revert * it locally -- cgit v0.10.2 From b642f7269bd40ae9abe9cff01581b2eb5e2c2287 Mon Sep 17 00:00:00 2001 From: "Kirill A. Shutemov" Date: Mon, 29 Dec 2014 15:00:19 +0200 Subject: 9p: do not crash on unknown lock status code Current 9p implementation will crash whole system if sees unknown lock status code. It's trivial target for DOS: 9p server can produce such code easily. Let's fallback more gracefully: warning in dmesg + -ENOLCK. Signed-off-by: Kirill A. Shutemov Signed-off-by: Dominique Martinet Signed-off-by: Eric Van Hensbergen diff --git a/fs/9p/vfs_file.c b/fs/9p/vfs_file.c index 8d29e1e..9612e5f 100644 --- a/fs/9p/vfs_file.c +++ b/fs/9p/vfs_file.c @@ -212,12 +212,13 @@ static int v9fs_file_do_lock(struct file *filp, int cmd, struct file_lock *fl) case P9_LOCK_BLOCKED: res = -EAGAIN; break; + default: + WARN_ONCE(1, "unknown lock status code: %d\n", status); + /* fallthough */ case P9_LOCK_ERROR: case P9_LOCK_GRACE: res = -ENOLCK; break; - default: - BUG(); } out_unlock: -- cgit v0.10.2 From 6250a8badb311953a49bedb16ed17eb59d21c03a Mon Sep 17 00:00:00 2001 From: "Kirill A. Shutemov" Date: Tue, 30 Dec 2014 02:48:09 +0200 Subject: 9p: use unsigned integers for nwqid/count As specification says, all integers in messages are unsigned. Let's fix behaviour of p9pdu_vreadf()/p9pdu_vwritef() accordingly. Fix for p9pdu_vreadf() is critical. If server replies with Rwalk, where nwqid > SHRT_MAX, the value will be interpreted as negative. kmalloc, in its order, will cast the value to (very big) size_t. It should never happen in normal situation: we never submit Twalk with nwname > 16, but malicious or broken server can still produce problematic Rwalk. Signed-off-by: Kirill A. Shutemov Signed-off-by: Dominique Martinet Signed-off-by: Eric Van Hensbergen diff --git a/net/9p/protocol.c b/net/9p/protocol.c index ab9127e..305e478 100644 --- a/net/9p/protocol.c +++ b/net/9p/protocol.c @@ -273,7 +273,7 @@ p9pdu_vreadf(struct p9_fcall *pdu, int proto_version, const char *fmt, } break; case 'R':{ - int16_t *nwqid = va_arg(ap, int16_t *); + uint16_t *nwqid = va_arg(ap, uint16_t *); struct p9_qid **wqids = va_arg(ap, struct p9_qid **); @@ -448,7 +448,7 @@ p9pdu_vwritef(struct p9_fcall *pdu, int proto_version, const char *fmt, } break; case 'U':{ - int32_t count = va_arg(ap, int32_t); + uint32_t count = va_arg(ap, uint32_t); const char __user *udata = va_arg(ap, const void __user *); errcode = p9pdu_writef(pdu, proto_version, "d", @@ -479,7 +479,7 @@ p9pdu_vwritef(struct p9_fcall *pdu, int proto_version, const char *fmt, } break; case 'R':{ - int16_t nwqid = va_arg(ap, int); + uint16_t nwqid = va_arg(ap, int); struct p9_qid *wqids = va_arg(ap, struct p9_qid *); -- cgit v0.10.2 From 179a5bc4b8cbe68ca675057b960dd805867e41c4 Mon Sep 17 00:00:00 2001 From: Andrey Ryabinin Date: Tue, 27 Jan 2015 16:00:19 +0300 Subject: net/9p: use memcpy() instead of snprintf() in p9_mount_tag_show() p9_mount_tag_show() uses '%s' format string to print non-NULL terminated chan->tag string. This leads to out of bounds memory read, because format '%s' implies that string is NULL-terminated. The length of string is know here, so its simpler and safer to use memcpy instead of snprintf(). Signed-off-by: Andrey Ryabinin Signed-off-by: Dominique Martinet Signed-off-by: Eric Van Hensbergen diff --git a/net/9p/trans_virtio.c b/net/9p/trans_virtio.c index 36a1a73..486df01 100644 --- a/net/9p/trans_virtio.c +++ b/net/9p/trans_virtio.c @@ -504,7 +504,10 @@ static ssize_t p9_mount_tag_show(struct device *dev, vdev = dev_to_virtio(dev); chan = vdev->priv; - return snprintf(buf, chan->tag_len + 1, "%s", chan->tag); + memcpy(buf, chan->tag, chan->tag_len); + buf[chan->tag_len] = 0; + + return chan->tag_len + 1; } static DEVICE_ATTR(mount_tag, 0444, p9_mount_tag_show, NULL); -- cgit v0.10.2 From b99baa43e533eb62a947e623d0ef844cfbf28d8e Mon Sep 17 00:00:00 2001 From: Dominique Martinet Date: Fri, 9 Jan 2015 13:05:56 +0100 Subject: net/9p: Initialize opts->privport as it should be. We're currently using an uninitialized value if option privport is not set, thus (almost) always using a privileged port. Signed-off-by: Dominique Martinet Signed-off-by: Eric Van Hensbergen diff --git a/net/9p/trans_fd.c b/net/9p/trans_fd.c index c73b894..154479d 100644 --- a/net/9p/trans_fd.c +++ b/net/9p/trans_fd.c @@ -734,6 +734,7 @@ static int parse_opts(char *params, struct p9_fd_opts *opts) opts->port = P9_PORT; opts->rfd = ~0; opts->wfd = ~0; + opts->privport = 0; if (!params) return 0; -- cgit v0.10.2 From 5c4086b8de6989f10ae814f5746604fc44a02a21 Mon Sep 17 00:00:00 2001 From: Dominique Martinet Date: Fri, 9 Jan 2015 12:56:07 +0100 Subject: fs/9p: Initialize status in v9fs_file_do_lock. If p9_client_lock_dotl returns an error, status is possibly never filled but will be used in the following switch. Initializing it to P9_LOCK_ERROR makes sur we will return an error and cleanup (and not hit the default case). Signed-off-by: Dominique Martinet Signed-off-by: Eric Van Hensbergen diff --git a/fs/9p/vfs_file.c b/fs/9p/vfs_file.c index 9612e5f..bdb103f 100644 --- a/fs/9p/vfs_file.c +++ b/fs/9p/vfs_file.c @@ -149,7 +149,7 @@ static int v9fs_file_do_lock(struct file *filp, int cmd, struct file_lock *fl) { struct p9_flock flock; struct p9_fid *fid; - uint8_t status; + uint8_t status = P9_LOCK_ERROR; int res = 0; unsigned char fl_type; -- cgit v0.10.2 From f569d3ef8254d4b3b8daa4f131f9397d48bf296c Mon Sep 17 00:00:00 2001 From: Dominique Martinet Date: Fri, 9 Jan 2015 13:07:00 +0100 Subject: net/9p: add a privport option for RDMA transport. RDMA can use the same kind of weak security as TCP by checking the client can bind to a privileged port, which is better than nothing if TAUTH isn't implemented. Signed-off-by: Dominique Martinet Signed-off-by: Eric Van Hensbergen diff --git a/net/9p/trans_rdma.c b/net/9p/trans_rdma.c index 14ad43b..3533d2a 100644 --- a/net/9p/trans_rdma.c +++ b/net/9p/trans_rdma.c @@ -139,6 +139,7 @@ struct p9_rdma_opts { int sq_depth; int rq_depth; long timeout; + int privport; }; /* @@ -146,7 +147,10 @@ struct p9_rdma_opts { */ enum { /* Options that take integer arguments */ - Opt_port, Opt_rq_depth, Opt_sq_depth, Opt_timeout, Opt_err, + Opt_port, Opt_rq_depth, Opt_sq_depth, Opt_timeout, + /* Options that take no argument */ + Opt_privport, + Opt_err, }; static match_table_t tokens = { @@ -154,6 +158,7 @@ static match_table_t tokens = { {Opt_sq_depth, "sq=%u"}, {Opt_rq_depth, "rq=%u"}, {Opt_timeout, "timeout=%u"}, + {Opt_privport, "privport"}, {Opt_err, NULL}, }; @@ -175,6 +180,7 @@ static int parse_opts(char *params, struct p9_rdma_opts *opts) opts->sq_depth = P9_RDMA_SQ_DEPTH; opts->rq_depth = P9_RDMA_RQ_DEPTH; opts->timeout = P9_RDMA_TIMEOUT; + opts->privport = 0; if (!params) return 0; @@ -193,13 +199,13 @@ static int parse_opts(char *params, struct p9_rdma_opts *opts) if (!*p) continue; token = match_token(p, tokens, args); - if (token == Opt_err) - continue; - r = match_int(&args[0], &option); - if (r < 0) { - p9_debug(P9_DEBUG_ERROR, - "integer field, but no integer?\n"); - continue; + if ((token != Opt_err) && (token != Opt_privport)) { + r = match_int(&args[0], &option); + if (r < 0) { + p9_debug(P9_DEBUG_ERROR, + "integer field, but no integer?\n"); + continue; + } } switch (token) { case Opt_port: @@ -214,6 +220,9 @@ static int parse_opts(char *params, struct p9_rdma_opts *opts) case Opt_timeout: opts->timeout = option; break; + case Opt_privport: + opts->privport = 1; + break; default: continue; } @@ -607,6 +616,23 @@ static int rdma_cancelled(struct p9_client *client, struct p9_req_t *req) return 0; } +static int p9_rdma_bind_privport(struct p9_trans_rdma *rdma) +{ + struct sockaddr_in cl = { + .sin_family = AF_INET, + .sin_addr.s_addr = htonl(INADDR_ANY), + }; + int port, err = -EINVAL; + + for (port = P9_DEF_MAX_RESVPORT; port >= P9_DEF_MIN_RESVPORT; port--) { + cl.sin_port = htons((ushort)port); + err = rdma_bind_addr(rdma->cm_id, (struct sockaddr *)&cl); + if (err != -EADDRINUSE) + break; + } + return err; +} + /** * trans_create_rdma - Transport method for creating atransport instance * @client: client instance @@ -642,6 +668,16 @@ rdma_create_trans(struct p9_client *client, const char *addr, char *args) /* Associate the client with the transport */ client->trans = rdma; + /* Bind to a privileged port if we need to */ + if (opts.privport) { + err = p9_rdma_bind_privport(rdma); + if (err < 0) { + pr_err("%s (%d): problem binding to privport: %d\n", + __func__, task_pid_nr(current), -err); + goto error; + } + } + /* Resolve the server's address */ rdma->addr.sin_family = AF_INET; rdma->addr.sin_addr.s_addr = in_aton(addr); -- cgit v0.10.2