netdev - [WTF?] random test in netlink

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [thread-next>] [day] [month] [year] [list]

Message-ID: <20141128062315.GC29748@ZenIV.linux.org.uk>
Date:	Fri, 28 Nov 2014 06:23:15 +0000
From:	Al Viro <viro@...IV.linux.org.uk>
To:	David Miller <davem@...emloft.net>
Cc:	Patrick McHardy <kaber@...sh.net>, netdev@...r.kernel.org
Subject: [WTF?] random test in netlink_sendmsg()

	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...
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html