Hi Neil:

On Wed, Sep 05, 2007 at 01:50:21PM +0100, Neil Brown wrote:
>
> > iov == NULL used to work.

Well it used to work mostly.  In fact, it still does work
mostly too.

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

Not quite.  If MSG_TRUNC is set, then we'll just call
udp_lib_checksum_complete.  If it fails we'll bail, if
it succeeds then skb_csum_unnecessary(skb) is guaranteed
to be true.

So in this respect the new code behaves identically with
the old code.

> 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", 

Nope, if len (== copied) is too small, we simply set MSG_TRUNC
as we did before.

Where this all falls apart is when the UDP payload is
empty (ulen == 0).  In that case MSG_TRUNC is not set
and we end up calling skb_copy_and_csum_datagram_iovec
which oopses.  I've looked up the original crash and
indeed %edi there which corresponds to the UDP payload
length is zero.

However, I can see where you're coming from in that we
shouldn't dereference msg_iov at all if msg_iovlen is 0.
This never happens with sys_recvmsg so we never bothered
checking it.  The following patch should fix the problem.

[NET]: Do not dereference iov if length is zero

When msg_iovlen is zero we shouldn't try to dereference
msg_iov.  Right now the only thing that tries to do so
is skb_copy_and_csum_datagram_iovec.  Since the total
length should also be zero if msg_iovlen is zero, it's
sufficient to check the total length there and simply
return if it's zero.

Signed-off-by: Herbert Xu <[EMAIL PROTECTED]>

Cheers,
-- 
Visit Openswan at http://www.openswan.org/
Email: Herbert Xu ~{PmV>HI~} <[EMAIL PROTECTED]>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt
--
diff --git a/net/core/datagram.c b/net/core/datagram.c
--- a/net/core/datagram.c
+++ b/net/core/datagram.c
@@ -450,6 +450,9 @@ int skb_copy_and_csum_datagram_iovec(str
        __wsum csum;
        int chunk = skb->len - hlen;
 
+       if (!chunk)
+               return 0;
+
        /* Skip filled elements.
         * Pretty silly, look at memcpy_toiovec, though 8)
         */
-
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Reply via email to