From: Al Viro <viro@ZenIV.linux.org.uk>
To: David Miller <davem@davemloft.net>
Cc: Patrick McHardy <kaber@trash.net>, netdev@vger.kernel.org
Subject: [WTF?] random test in netlink_sendmsg()
Date: Fri, 28 Nov 2014 06:23:15 +0000 [thread overview]
Message-ID: <20141128062315.GC29748@ZenIV.linux.org.uk> (raw)
In netlink_sendmsg() we have the following:
if (netlink_tx_is_mmaped(sk) &&
msg->msg_iov->iov_base == NULL) {
err = netlink_mmap_sendmsg(sk, msg, dst_portid, dst_group,
siocb);
goto out;
}
Now, suppose sendmsg(2) is called with msg.msg_iovlen == 0. We'll have
->msg_iov in kernel-side copy pointing at the uninitialized array in
stack frame of ___sys_sendmsg() - neither new nor old code touches elements
past the first msg_iovlen ones. So in that case it checks if an
uninitialized word on stack is zero.
What is that check trying to do? Is that simply missing
"(msg->msg_iovlen > 0) &&"? And why on the Earth didn't it simply use
zero msg_iovlen as the indicator, instead of messing with iovec contents?
Obviously too late to change, but... ouch.
Patrick, it had been that way since your commit last year ("netlink: implement
memory mapped sendmsg()"); could you explain what's the intended ABI?
Incidentally, WTF is "atomic_read(&nlk->mapped) > 1" part of check in
netlink_mmap_sendmsg() trying to achieve? AFAICS, ->mapped tries to
keep track of the number of VMAs, right? If so, it's bloody pointless -
one can have memory accessible in more than one process without any
extra VMAs. Just clone(2) with CLONE_VM. Voila - child shares the
entire address space. No extra VMAs or calls of ->open() in sight...
next reply other threads:[~2014-11-28 6:23 UTC|newest]
Thread overview: 12+ messages / expand[flat|nested] mbox.gz Atom feed top
2014-11-28 6:23 Al Viro [this message]
2014-12-12 20:34 ` [WTF?] random test in netlink_sendmsg() David Miller
2014-12-12 21:32 ` Al Viro
2014-12-12 21:50 ` Florian Westphal
2014-12-12 22:14 ` Al Viro
2014-12-12 22:20 ` Florian Westphal
2014-12-13 1:07 ` David Miller
2014-12-13 1:54 ` Al Viro
2014-12-13 2:33 ` David Miller
2014-12-13 3:25 ` Al Viro
2014-12-13 4:51 ` Al Viro
2014-12-14 4:38 ` David Miller
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=20141128062315.GC29748@ZenIV.linux.org.uk \
--to=viro@zeniv.linux.org.uk \
--cc=davem@davemloft.net \
--cc=kaber@trash.net \
--cc=netdev@vger.kernel.org \
/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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.