All of lore.kernel.org
 help / color / mirror / Atom feed
From: Al Viro <viro@ZenIV.linux.org.uk>
To: David Miller <davem@davemloft.net>
Cc: kaber@trash.net, netdev@vger.kernel.org
Subject: Re: [WTF?] random test in netlink_sendmsg()
Date: Fri, 12 Dec 2014 21:32:43 +0000	[thread overview]
Message-ID: <20141212213242.GE22149@ZenIV.linux.org.uk> (raw)
In-Reply-To: <20141212.153433.1675057029307550538.davem@davemloft.net>

On Fri, Dec 12, 2014 at 03:34:33PM -0500, David Miller wrote:
> > 	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.
> 
> I think it's simply trying to say: if nothing in the given iovec, use
> the mmap() netlink area for the data.
> 
> I cannot vouch for the correctness of this test.
> 
> If we take the netlink_mmap_sendmsg() path, msg->msg_iov is not
> accessed at all, so it cannot be a huge problem.

Yes, but decision whether to take that path or not is random in case of
msg_iovlen being 0...

What do we want sendmsg(fd, &msg, 0) to do when fd is AF_NETLINK socket
that had setsockopt(fd, SOL_NETLINK, NETLINK_TX_RING, ...) successfully done
to it and msg.msg_iovlen is 0?  Userland ABI question - not a "do we have
a security hole there" one...  As it is, it might decide to do
netlink_mmap_sendmsg() or it might decide to act as if we hadn't done
NETLINK_TX_RING at all.  In _both_ cases it won't try to dereference
kernel-side ->msg_iov[0].iov_base - the former won't look at msg_iov at all,
the latter will ask to copy 0 bytes from it, which will succeed and do nothing
whatsoever.  Which path is taken depends on whether an uninitialized array
on stack frame of ___sys_sendmsg() happens to begin with NULL.

IMO that's bogus - behaviour ought to be at least deterministic...

  reply	other threads:[~2014-12-12 21:32 UTC|newest]

Thread overview: 12+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2014-11-28  6:23 [WTF?] random test in netlink_sendmsg() Al Viro
2014-12-12 20:34 ` David Miller
2014-12-12 21:32   ` Al Viro [this message]
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=20141212213242.GE22149@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.