netdev - Re: [PATCH 15/17] switch kernel_sendmsg() and kernel_recvmsg() to iov_iter

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-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20150414165920.GB889@ZenIV.linux.org.uk>
Date:	Tue, 14 Apr 2015 17:59:21 +0100
From:	Al Viro <viro@...IV.linux.org.uk>
To:	David Laight <David.Laight@...LAB.COM>
Cc:	"davem@...emloft.net" <davem@...emloft.net>,
	"netdev@...r.kernel.org" <netdev@...r.kernel.org>
Subject: Re: [PATCH 15/17] switch kernel_sendmsg() and kernel_recvmsg() to
 iov_iter_kvec()

On Tue, Apr 14, 2015 at 04:36:36PM +0000, David Laight wrote:
> From: Al Viro 
> > Sent: 14 April 2015 17:34
> > On Tue, Apr 14, 2015 at 04:21:02PM +0000, David Laight wrote:
> > 
> > > Massive NAK.
> > > This breaks any code that is using msg_control to set SCTP parameters
> > > when sending data.
> > 
> > 	Huh?  ->sendmsg() expects ->msg_control already in kernel space;
> > it's ->recvmsg() that plays silly buggers with userland pointers there.
> 
> I read your commit message as implying that you hadn't found any
> users of kernel_sendmsg() that used msg_control.
> Not that the data was always read from kernel space.

Sigh...  The situation is:
	* ->sendmsg() expects ->msg_control copied to userland.  sendmsg(2),
sendto(2), etc. do that copying.  See ___sys_sendmsg() - there we have
                /*
                 * Careful! Before this, msg_sys->msg_control contains a user pointer.
                 * Afterwards, it will be a kernel pointer. Thus the compiler-assisted
                 * checking falls down on this.
                 */
                if (copy_from_user(ctl_buf,
                                   (void __user __force *)msg_sys->msg_control,
                                   ctl_len))
                        goto out_freectl;
                msg_sys->msg_control = ctl_buf;
As the result, ->sendmsg() instances access ->msg_control contents as normal
kernel data.
	* ->recvmsg() expects ->msg_control to point to userland.  See
net/core/scm.c for the helpers used to store into it.  recvmsg(2) et.al.
simply leave the userland pointer there; worse, that pointer might be
to native or to compat variants, and layouts _are_ different.  Thus those
if (MSG_CMSG_COMPAT & msg->msg_flags) in net/core/scm.c...
	* kernel-side users of ->sendmsg() do not depend on setfs() for
access to their ->msg_control, simply because ->sendmsg() won't be using
copy_from_user()/get_user() to access it anyway.
	* kernel-side users of ->recvmsg() are less lucky - most of them
don't give a damn either (they have NULL ->msg_control), but there's an
exception (somewhere in sunrpc, IIRC).  So there we need to keep
playing with setfs(), even though the data side would be just fine without
that.
--
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