netdev - Re: Oops in 2.6.22.1: skb_copy_and_csum_datagram

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 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]

Date:	Wed, 5 Sep 2007 13:50:21 +0100
From:	Neil Brown <neilb@...e.de>
To:	Herbert Xu <herbert@...dor.apana.org.au>,
	cebbert@...hat.com (Chuck Ebbert), netdev@...r.kernel.org
Subject: Re: Oops in 2.6.22.1: skb_copy_and_csum_datagram_iovec()

On Wednesday September 5, neilb@...e.de wrote:
> On Wednesday August 22, herbert@...dor.apana.org.au wrote:
> > Chuck Ebbert <cebbert@...hat.com> wrote:
> > > https://bugzilla.redhat.com/bugzilla/show_bug.cgi?id=253290
> > > 
> > > 18:57:54 osama kernel:  [<c05be67f>] kernel_recvmsg+0x31/0x40
> > > 18:57:54 osama kernel:  [<e0bc52d4>] svc_udp_recvfrom+0x114/0x368 [sunrpc]
> > 
> > svc_udp_recvfrom is calling kernel_recvmsg with iov == NULL.
> 
> iov == NULL used to work.
> 
> I think it stopped working at 
>    commit 759e5d006462d53fb708daa8284b4ad909415da1
> 
> Previously, as len==0, MSG_TRUNC would get set, so copy_only would get
> set, so skb_copy_datagram_iovec would get called, and that handles a
> len of 0.
> 
> Now, skb_copy_and_csum_datagram_iovec gets called unless
> skb_csum_unnecessary(skb), which now kills us.

Actually, the new code is broken for more reasons than that.
In core/datagram.c, the comment for skb_copy_and_csum_datagram_iovec,
it says:
 *	Caller _must_ check that skb will fit to this iovec.

but udp_recvmsg doesn't.

It seems to try:
	if (copied < ulen || UDP_SKB_CB(skb)->partial_cov) {
		if (udp_lib_checksum_complete(skb))
			goto csum_copy_err;
	}

	if (skb_csum_unnecessary(skb))
		err = skb_copy_datagram_iovec(skb, sizeof(struct udphdr),
					      msg->msg_iov, copied       );

so it doesn't call skb_copy_datagram_iovec if "copied < ulen".

However earlier there is:
	ulen = skb->len - sizeof(struct udphdr);
	copied = len;
	if (copied > ulen)
		copied = ulen;

so if the 'len' (of the iovec) is too small, we end up with "copied == ulen", 

so udp_lib_checksum_complete doesn't get called....


> 
> We could 'fix' it by making skb_copy_and_csum_datagram_iovec just
> return if len==0, or don't call it from udp_recvmsg in that case.
> 

So the latter of these is needed.

NeilBrown
-
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