Problem with zcip's packing of ARP packets?

Patrick Noffke Patrick.Noffke at adpro.com.au
Fri Aug 25 08:25:38 UTC 2006


> -----Original Message-----
> From: busybox-bounces at busybox.net 
> [mailto:busybox-bounces at busybox.net]On
> Behalf Of Rich Felker
> Sent: Friday, 25 August 2006 3:21 PM
> To: busybox at busybox.net
> Subject: Re: Problem with zcip's packing of ARP packets?
> 
> 
> On Thu, Aug 24, 2006 at 09:55:08PM -0700, Andre wrote:
> > --- Jason Schoon <floydpink at gmail.com> wrote:
> > > 
> > > I have seen this problem as well, and submitted a patch 
> to the list
> > > some time ago.  I don't believe it ever got applied though.
> > > Here it is again.
> > 
> > Patch looks good except for this:
> > 
> > > +   /* This cast is safe because ip is always kept as BE */
> > > +   if ((*(unsigned long *)p.arp.arp_spa == ip.s_addr)
> > 
> > The cast is NOT safe (but not for the reason you mention... ;-).
> > 
> > p is a packed structure, therefore arp_spa is not necessarily 32bit
> > aligned... which means it can't be accessed via an unsigned long
> > pointer on architectures which care about alignment...
> 
> It's not being accessed as an unsigned long pointer. It's being
> accessed as whatever type the arp_spa member is, and then _cast_ to a
> pointer to unsigned long. However the code looks wrong. I suspect it
> was intended to do what you said, i.e. the intent was:
> 
> if ((*(unsigned long *)&p.arp.arp_spa == ip.s_addr)
> 
> and this of course is invalid if arp_spa is not aligned. Like you say,
> memcmp would be the correct approach.
> 

I have updated my patch with the state machine implementation according to the above advice.  New patch is against 1.2.1 and is attached.  I'll update issue 1005 as well.

Pat
-------------- next part --------------
A non-text attachment was scrubbed...
Name: busybox-1.2.1-zcip.patch
Type: application/octet-stream
Size: 11128 bytes
Desc: busybox-1.2.1-zcip.patch
Url : http://lists.busybox.net/pipermail/busybox/attachments/20060825/07b02e4a/attachment-0002.obj 


More information about the busybox mailing list