From: "Kirill A. Shutemov" <kirill.shutemov@linux.intel.com>
To: Eric Van Hensbergen <ericvh@gmail.com>,
Ron Minnich <rminnich@sandia.gov>,
Latchesar Ionkov <lucho@ionkov.net>
Cc: v9fs-developer@lists.sourceforge.net,
linux-fsdevel@vger.kernel.org,
"Kirill A. Shutemov" <kirill.shutemov@linux.intel.com>
Subject: [PATCH] 9p: use unsinged integers for nwqid/count
Date: Tue, 30 Dec 2014 02:48:09 +0200 [thread overview]
Message-ID: <1419900489-44041-1-git-send-email-kirill.shutemov@linux.intel.com> (raw)
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:
[ 16.330386] ------------[ cut here ]------------
[ 16.331479] WARNING: CPU: 10 PID: 239 at /home/kas/git/public/linux-next/mm/page_alloc.c:2660 __alloc_pages_nodemask+0x4c9/0xb80()
[ 16.333039] Modules linked in:
[ 16.333433] CPU: 10 PID: 239 Comm: trinity-c62 Not tainted 3.19.0-rc1-next-20141226-00040-g0bb79cfd1d6c-dirty #388
[ 16.334804] Hardware name: QEMU Standard PC (Q35 + ICH9, 2009), BIOS rel-1.7.5.1-0-g8936dbb-20141113_115728-nilsson.home.kraxel.org 04/01/2014
[ 16.337407] ffffffff81c7e8e8 ffff88046bf77978 ffffffff81854c3a 0000000000000c52
[ 16.339154] 0000000000000000 ffff88046bf779b8 ffffffff8105adda 0000000000000b3c
[ 16.340702] 0000000000000000 ffff880495f0df80 0000000000000000 ffff880495f0ef00
[ 16.342414] Call Trace:
[ 16.342996] [<ffffffff81854c3a>] dump_stack+0x4c/0x65
[ 16.344129] [<ffffffff8105adda>] warn_slowpath_common+0x8a/0xc0
[ 16.345312] [<ffffffff8105aeca>] warn_slowpath_null+0x1a/0x20
[ 16.346644] [<ffffffff811560c9>] __alloc_pages_nodemask+0x4c9/0xb80
[ 16.347876] [<ffffffff81092385>] ? sched_clock_local+0x25/0x90
[ 16.348896] [<ffffffff81086a57>] ? finish_task_switch+0x87/0x110
[ 16.350106] [<ffffffff8119ffbb>] alloc_pages_current+0x6b/0xc0
[ 16.351299] [<ffffffff81151ade>] alloc_kmem_pages+0xe/0x10
[ 16.352293] [<ffffffff81173458>] kmalloc_order+0x18/0x50
[ 16.353348] [<ffffffff811734b4>] kmalloc_order_trace+0x24/0x210
[ 16.354528] [<ffffffff811ad9e9>] __kmalloc+0x2b9/0x3b0
[ 16.355623] [<ffffffff81841daa>] p9pdu_vreadf+0x57a/0x9b0
[ 16.356392] [<ffffffff810a84ce>] ? __lock_is_held+0x5e/0x90
[ 16.357142] [<ffffffff8183b4c3>] ? p9_client_rpc+0x813/0x840
[ 16.358025] [<ffffffff81841829>] p9pdu_readf+0x39/0x40
[ 16.358780] [<ffffffff8183ede2>] p9_client_walk+0xc2/0x400
[ 16.359699] [<ffffffff811adab8>] ? __kmalloc+0x388/0x3b0
[ 16.360755] [<ffffffff8139eb75>] ? v9fs_fid_lookup_with_uid+0x195/0x310
[ 16.361123] VFS: Warning: trinity-c56 using old stat() call. Recompile your binary.
[ 16.363903] [<ffffffff8139ec0f>] v9fs_fid_lookup_with_uid+0x22f/0x310
[ 16.365422] [<ffffffff8139ed5b>] v9fs_fid_lookup+0x6b/0x80
[ 16.366815] [<ffffffff813985ab>] v9fs_statfs+0x1b/0xc0
[ 16.368185] [<ffffffff811f6c82>] statfs_by_dentry+0x62/0x90
[ 16.369634] [<ffffffff811f6e4b>] vfs_statfs+0x1b/0xb0
[ 16.370951] [<ffffffff811f6fa3>] fd_statfs+0x33/0x60
[ 16.372075] [<ffffffff811f7090>] SyS_fstatfs+0x20/0x40
[ 16.373362] [<ffffffff8185e2b7>] ? sysret_check+0x1b/0x56
[ 16.374576] [<ffffffff8185e292>] system_call_fastpath+0x12/0x17
[ 16.375815] ---[ end trace 949914f1bb21cfd4 ]---
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 <kirill.shutemov@linux.intel.com>
---
net/9p/protocol.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/net/9p/protocol.c b/net/9p/protocol.c
index ab9127ec5b7a..305e4789f2cc 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 *);
--
2.1.4
next reply other threads:[~2014-12-30 0:48 UTC|newest]
Thread overview: 3+ messages / expand[flat|nested] mbox.gz Atom feed top
2014-12-30 0:48 Kirill A. Shutemov [this message]
2015-01-06 13:04 ` [V9fs-developer] [PATCH] 9p: use unsinged integers for nwqid/count Dominique Martinet
2015-01-07 12:06 ` Kirill A. Shutemov
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=1419900489-44041-1-git-send-email-kirill.shutemov@linux.intel.com \
--to=kirill.shutemov@linux.intel.com \
--cc=ericvh@gmail.com \
--cc=linux-fsdevel@vger.kernel.org \
--cc=lucho@ionkov.net \
--cc=rminnich@sandia.gov \
--cc=v9fs-developer@lists.sourceforge.net \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).