[net.unix-wizards] recvfrom

Larry Allen <lwa@mit-mrclean.ARPA> (11/10/84)

Description:
	Recvfrom occasionally (about once out of every 1000 packets received)
	returns a bad source address when used on a raw socket.  The packet
	is received successfully, everything looks fine, but the contents of
	the from sockaddr are completely trashed.
Repeat-By:
	Because it happens so rarely, this problem is a hard one to repeat.
Fix:
	The problem is caused by a bug in the rawintr() procedure in the
	file /sys/net/raw_usrreq.c.  An extra mbuf has been prepended to 
	the received packet to hold the demultiplexing information; the
	address of the source address in this mbuf is passed to sbappendaddr.
	However, the header mbuf is mfree'd before the call to sbappendaddr,
	and under heavy network traffic it may be reused before the address
	can be copied from it.

	Make the following changes to rawintr():

*** /fs/usr/sys/net/raw_usrreq.c	Fri Mar 23 03:15:38 1984
--- /u/sys/net/raw_usrreq.c	Fri Nov  9 16:08:12 1984
***************
*** 125,133
  		last = rp->rcb_socket;
  	}
  	if (last) {
- 		m = m_free(m);		/* header */
  		if (sbappendaddr(&last->so_rcv, &rh->raw_src,
! 		    m, (struct mbuf *)0) == 0)
  			goto drop;
  		sorwakeup(last);
  		goto next;

--- 126,132 -----
  	}
  	if (last) {
  		if (sbappendaddr(&last->so_rcv, &rh->raw_src,
! 		    m->m_next, (struct mbuf *)0) == 0)
  			goto drop;
+  		m_free(m);		/* header */
  		sorwakeup(last);

******************
					-Larry Allen

steveh@hammer.UUCP (Stephen Hemminger) (11/16/84)

The bug fix you suggested is close, but incorrect.

The sbappendaddr routine free's its input chain if it is sucessful (returns 1)
If it is unsucessful, the data chain is not freed.  Therefore the call
to m_free is not needed, and may cause panic freeing free mbuf.

Larry Allen <lwa@mit-mrclean.ARPA> (11/18/84)

I think the diffs in my message may have confused you.  Here's the
final version of the code:

	if (last) {
		if (sbappendaddr(&last->so_rcv, &rh->raw_src,
		    m->m_next, (struct mbuf *)0) == 0)
			goto drop;
		m_free(m);		/* header */
		sorwakeup(last);
		goto next;
	}
drop:
	m_freem(m);
	goto next;

Notice that sbappendaddr is called with m->m_next as its pointer; thus
it frees everything in the chain EXCEPT the header mbuf (containing
the addressing information) if successful.  On error, it doesn't
free anything, so the entire chain (beginning at m) should be freed.
					-Larry Allen

chris@umcp-cs.UUCP (Chris Torek) (11/18/84)

> The bug fix you suggested is close, but incorrect.

> The sbappendaddr routine free's its input chain if it is sucessful
> (returns 1) If it is unsucessful, the data chain is not freed.
> Therefore the call to m_free is not needed, and may cause panic
> freeing free mbuf.

Are we looking at the same code?

The m_free is freeing the *header*, not the *data chain*; the reason
for moving the call was that &rhp->rh_src (wonder if I remembered that
right) is right in the mbuf ``m'', so the m_free must be moved down
a bit.

That sbappendaddr returns 1 for success and 0 for failure, rathern
than 0 for success and -1 (or an error code) for failure, is another
matter entirely....
-- 
(This line accidently left nonblank.)

In-Real-Life: Chris Torek, Univ of MD Comp Sci Dept (301) 454-7690
UUCP:	{seismo,allegra,brl-bmd}!umcp-cs!chris
CSNet:	chris@umcp-cs		ARPA:	chris@maryland