> On Sat, Aug 07, 2004 at 06:04:28PM +0200, Pavel Cahyna wrote:
> > is it correct to call mtod() without calling m_pullup() before dereferencing
> > the pointer obtained? I would think that it isn't. Such calls do occur in
> > wi.c, function wi_start(), however. e.g.
> > 
> > --- cut here ---
> > 			IF_DEQUEUE(&ic->ic_pwrsaveq, m0);
> >                         wh = mtod(m0, struct ieee80211_frame *);
> > 			llc = (struct llc *) (wh + 1);
> > 			m_copydata(m0, 4, ETHER_ADDR_LEN * 2,
> > 			    (caddr_t)&frmhdr.wi_ehdr);
> > --->			frmhdr.wi_ehdr.ether_type = llc->llc_snap.ether_type;
> > --- cut here ---
> 
> This looks like a bug to me.  I am looking at ieee80211_encap, which
> inserts the LLC header, and I do not think the 802.11 header and LLC
> header will ordinarily be stored consecutively.
> 
> An m_pullup is appropriate.  However, I do not see how the LLC header
> can straddle two mbufs.  So I recommend that you use m_pulldown instead
> of m_pullup.

I do not really understand this ... even if the header straddled two
mbufs, m_pulldown could be used, as it is supposed to correct this, no?

> > --- cut here ---
> > 			IF_DEQUEUE(&ic->ic_mgtq, m0);
> > 			m_copydata(m0, 4, ETHER_ADDR_LEN * 2,
> > 			    (caddr_t)&frmhdr.wi_ehdr);
> > 			frmhdr.wi_ehdr.ether_type = 0;
> >                         wh = mtod(m0, struct ieee80211_frame *);
> > --- cut here ---
> 
> Again, I do not expect for ieee80211_frame to straddle more than one mbuf.

Yes, because wi_rx_intr puts whole packet in one mbuf. And ieee80211_input
can probably be trusted to not break it. So this piece looks not buggy,
but relying on IMHO quite fragile assumptions. Cuold you put at least a
comment there, please?

Thanks	Pavel