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--