Bug 70988

Summary: Bug in netisr_queue()
Product: Base System Reporter: MOROHOSHI Akihiko <moro>
Component: kernAssignee: Andre Oppermann <andre>
Status: Closed FIXED    
Severity: Affects Only Me    
Priority: Normal    
Version: 5.3-BETA1   
Hardware: Any   
OS: Any   
Attachments:
Description Flags
file.diff none

Description MOROHOSHI Akihiko 2004-08-26 14:40:24 UTC
In short:
netisr_queue() has a bug of its return value.  Please apply the patch.

Background:
When I used ipfw fwd action to realize transparent HTTP proxy,
it worked fine for other machines, but trying "telnet www.foobar.com 80"
on the FreeBSD box resulted in "Operation not permitted."
(Configurations were proven to work fine on 4-stable.
I've been migrating to 5.3-BETA1.)

Analysis:
The reason of the error EPERM(=1) is that ip_output() return 1,
and it is because netisr_queue() return 1.
I observed netisr_queue() always returned 1, even when forwarding
works fine for client machines.

Look at netisr_queue() in sys/net/netisr.c:
| int
| netisr_queue(int num, struct mbuf *m)
| {
| 	struct netisr *ni;
| 	
| 	KASSERT(!(num < 0 || num >= (sizeof(netisrs)/sizeof(*netisrs))),
| 	    ("bad isr %d", num));
| 	ni = &netisrs[num];
| 	if (ni->ni_queue == NULL) {
| 		isrstat.isrs_drop++;
| 		m_freem(m);
| 		return (1);
| 	}
| 	isrstat.isrs_queued++;
| 	if (!IF_HANDOFF(ni->ni_queue, m, NULL))
| 		return (0);
| 	schednetisr(num);
| 	return (1);
| }

In the last line netisr_queue returns 1, but it should return 0.

Also, it seems that netisr_queue assumes IF_HANDOFF returns 0 when succeeded,
but it is wrong. IF_HANDOFF returns 0 when the queue is full, and returns
1 when succeeded:

| #define	IF_HANDOFF(ifq, m, ifp)			\
| 	if_handoff((struct ifqueue *)ifq, m, ifp, 0)

| static __inline int
| if_handoff(struct ifqueue *ifq, struct mbuf *m, struct ifnet *ifp, int adjust)
| {
| 	int active = 0;
| 
| 	IF_LOCK(ifq);
| 	if (_IF_QFULL(ifq)) {
| 		_IF_DROP(ifq);
| 		IF_UNLOCK(ifq);
| 		m_freem(m);
| 		return (0);
| 	}
| 	if (ifp != NULL) {
| 		ifp->if_obytes += m->m_pkthdr.len + adjust;
| 		if (m->m_flags & (M_BCAST|M_MCAST))
| 			ifp->if_omcasts++;
| 		active = ifp->if_flags & IFF_OACTIVE;
| 	}
| 	_IF_ENQUEUE(ifq, m);
| 	IF_UNLOCK(ifq);
| 	if (ifp != NULL && !active)
| 		if_start(ifp);
| 	return (1);
| }

So, this part in netisr_queue() should be changed to return 1:
| 	if (!IF_HANDOFF(ni->ni_queue, m, NULL))
| 		return (0);

How-To-Repeat: 
1. Set up squid as a transparent HTTP proxy in jail (192.168.79.21).
2. Add ipfw rules for forwarding:
 ipfw add allow tcp from 192.168.79.21 to any 80 out setup keep-state
 ipfw add fwd 192.168.79.21,8080 tcp from any to any 80 out setup keep-state
3. Browse some web sites with client machines. It should be OK.
4. Try "telnet www.example.com 80" on the FreeBSD box.
   You will see "Operation not permitted."
Comment 1 Tilman Keskinoz freebsd_committer freebsd_triage 2004-08-26 22:30:57 UTC
Responsible Changed
From-To: freebsd-bugs->freebsd-net

Over to freebsd-net for review
Comment 2 Andre Oppermann freebsd_committer freebsd_triage 2004-08-26 22:36:44 UTC
Responsible Changed
From-To: freebsd-net->andre

Take over.
Comment 3 Andre Oppermann freebsd_committer freebsd_triage 2004-08-27 19:36:16 UTC
State Changed
From-To: open->patched

Hello Akihiko-san, 

I have committed your fix plus adjustments to all users of netisr_queue(). 

Revision 1.11 of sys/net/netisr.c contains the fix. 

Many thanks for your very good analysis of the problem! 

--  
Andre
Comment 4 Andre Oppermann freebsd_committer freebsd_triage 2004-09-16 21:36:19 UTC
State Changed
From-To: patched->closed

MFC to RELENG_5 done.  Case closed.