Subject: Re: ieee80211_mbuf_adjust
To: None <tech-net@NetBSD.org>
From: David Young <dyoung@pobox.com>
List: tech-net
Date: 08/16/2005 13:58:17
--kvUQC+jR9YzypDnK
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
On Mon, Aug 15, 2005 at 10:18:13PM -0500, David Young wrote:
> On Tue, Aug 16, 2005 at 02:23:21AM +0000, Christos Zoulas wrote:
> > In article <20050816021259.101BF2DA27@cvs.netbsd.org>,
> > David Young <dyoung@netbsd.org> wrote:
> > >
> > >Module Name: src
> > >Committed By: dyoung
> > >Date: Tue Aug 16 02:12:59 UTC 2005
> > >
> > >Modified Files:
> > > src/sys/net80211: ieee80211_output.c
> > >
> > >Log Message:
> > >Fix previous patch for non-crypto operation: test for a NULL key
> > >before testing the key flags.
> > >
> > >XXX Problems remain. Nick Hudson points out my questionable
> > >XXX M_COPY_PKTHDR usage. Also, it seems to me that we may not be
> > >XXX protected against writing a read-only mbuf during the crypto
> > >XXX encapsulation stage, even if hardware does the actual crypto.
> >
> > There is also the questionably usage of m_pullup and M_PREPEND in
> > the code. Yes, the code makes sure that there is adequate space
> > so that neither M_PREPEND or m_pullup will need to allocate a new
> > mbuf, but this is not guaranteed (their implementation might change).
> > I think that we either should change the api to pass an mbuf ** so
> > that changes are propagated, or add KASSERTS to the code.
>
> Let's KASSERT for now. API changes need to be coordinated with FreeBSD.
>
> I have attached a patch to ieee80211_mbuf_adjust that makes sure the
Oops. Here is the patch.
Dave
--
David Young OJC Technologies
dyoung@ojctech.com Urbana, IL * (217) 278-3933
--kvUQC+jR9YzypDnK
Content-Type: text/plain; charset=us-ascii
Content-Disposition: attachment; filename=mbuf-adj-patch
Index: sys/net80211/ieee80211_output.c
===================================================================
RCS file: /cvsroot/src/sys/net80211/ieee80211_output.c,v
retrieving revision 1.35
diff -u -u -r1.35 ieee80211_output.c
--- sys/net80211/ieee80211_output.c 16 Aug 2005 02:12:58 -0000 1.35
+++ sys/net80211/ieee80211_output.c 16 Aug 2005 03:05:31 -0000
@@ -352,7 +352,7 @@
{
#define TO_BE_RECLAIMED (sizeof(struct ether_header) - sizeof(struct llc))
int needed_space = hdrsize;
- int error;
+ int wlen = 0;
if (key != NULL) {
/* XXX belongs in crypto code? */
@@ -404,22 +404,26 @@
*/
n->m_next = m;
m = n;
+ } else {
+ /* Make sure the 802.11 header + crypto header + LLC is
+ * writable.
+ */
+ wlen = needed_space + sizeof(struct llc);
}
/*
* If we're going to s/w encrypt the mbuf chain make sure it is
* writable.
*/
- if (key != NULL && (key->wk_flags & IEEE80211_KEY_SWCRYPT) != 0) {
- error = m_makewritable(&m, 0, M_COPYALL, M_DONTWAIT);
+ if (key != NULL && (key->wk_flags & IEEE80211_KEY_SWCRYPT) != 0)
+ wlen = M_COPYALL;
- if (error) {
- m_freem(m);
- m = NULL;
- }
+ if (wlen == 0 || m_makewritable(&m, 0, wlen, M_DONTWAIT) == 0)
+ return m;
+ else {
+ m_freem(m);
+ return NULL;
}
-
- return m;
#undef TO_BE_RECLAIMED
}
--kvUQC+jR9YzypDnK--