Re: UDP checksum broken, -head and releng_8

On Sat, 8 Jan 2011, Daniel Eischen wrote:

>>>>> When sending multicast packets to a socket that is _not_
>>>>> bound to the multicast address, this generates bad UDP
>>>>> checksums.  This use to work and was broke sometime between
>>>>> the middle of October and late December as far as I can
>>>>> tell.
>>>> 
>>>> My very best guess would be: r215110
>>> 
>>> It doesn't look very harmful, but I'll try backing it out.
>> 
>> Backing this out seems to fix it.  I'll have to test it
>> more after I get some sleep ;-)
>
> I've attached what may be a proper patch.  Please review.
> I'd like to get this fixed in releng_8 too.

I would remove the comment from the MC good path about the in_pcbladdr()
error but just change the description at the top s,use,prefer, to be
more exact.

The other question I am not sure what shoud happen is, in the case
in_pcbladdr() returns a source address but a given one via MC option/ifp
does not find an address (in case outgoing itnerface could be different)?
That was never considered in the past.

It's probably easiest understood with the slightly modified version
of the patch.

Otherwise I think it would match both the historic behaviour again
and keep the fix for r215110 (that rev. should be mentioned in the
commit message).

So apart from the 1 line comment change (ignoring my XXX-BZ completely
for the moment and this fix) this looks good.

/bz

PS: I doubt you can make 8.2-R anymore.

Index: in_pcb.c
===================================================================
--- in_pcb.c	(revision 216952)
+++ in_pcb.c	(working copy)
_at__at_ -874,9 +874,10 _at__at_ in_pcbconnect_setup(struct inpcb *inp, struct sock
  		}
  	}
  	if (laddr.s_addr == INADDR_ANY) {
+		error = in_pcbladdr(inp, &faddr, &laddr, cred);
  		/*
  		 * If the destination address is multicast and an outgoing
-		 * interface has been set as a multicast option, use the
+		 * interface has been set as a multicast option, prefer the
  		 * address of that interface as our source address.
  		 */
  		if (IN_MULTICAST(ntohl(faddr.s_addr)) &&
_at__at_ -893,16 +894,23 _at__at_ in_pcbconnect_setup(struct inpcb *inp, struct sock
  						break;
  				if (ia == NULL) {
  					IN_IFADDR_RUNLOCK();
-					return (EADDRNOTAVAIL);
+					/*
+					 * XXX-BZ should we use a possible
+					 * source address from in_pcbladdr()
+					 * and only overwrite the error if
+					 * there was one?
+					 * if (error)
+					 */
+					error = EADDRNOTAVAIL;
+				} else {
+					laddr = ia->ia_addr.sin_addr;
+					IN_IFADDR_RUNLOCK();
+					error = 0;
  				}
-				laddr = ia->ia_addr.sin_addr;
-				IN_IFADDR_RUNLOCK();
  			}
-		} else {
-			error = in_pcbladdr(inp, &faddr, &laddr, cred);
-			if (error) 
-				return (error);
  		}
+		if (error)
+			return (error);
  	}
  	oinp = in_pcblookup_hash(inp->inp_pcbinfo, faddr, fport, laddr, lport,
  	    0, NULL);


-- 
Bjoern A. Zeeb                                 You have to have visions!
         <ks> Going to jail sucks -- <bz> All my daemons like it!
   http://www.freebsd.org/doc/en_US.ISO8859-1/books/handbook/jails.html