udhcpc alignment problem

Denis Vlasenko vda.linux at googlemail.com
Thu Jan 18 15:27:27 UTC 2007


On Thursday 18 January 2007 16:21, Peter Kjellerstedt wrote:
> > -----Original Message-----
> > From: busybox-bounces at busybox.net 
> > [mailto:busybox-bounces at busybox.net] On Behalf Of Denis Vlasenko
> > Sent: Thursday, January 18, 2007 15:54
> > To: busybox at busybox.net
> > Subject: Re: udhcpc alignment problem
> > 
> > On Thursday 18 January 2007 15:11, Kim B. Heino wrote:
> > > Hello,
> > > 
> > > There's an alignment problem in BusyBox 1.3.1's udhcpc 
> > > code. There are following two lines in dhcpc.c:
> > > 
> > >    server_addr = *(uint32_t*)temp;
> > > 
> > > and
> > > 
> > >    lease = ntohl(*(uint32_t*)temp);
> > > 
> > > Unfortunately temp is not aligned to 4 byte boundary as 
> > > required by some CPUs. The fix is either to do "echo 3 > 
> > > /proc/cpu/alignment" in userland, or to revert changes by 
> > > revision 16066:
> > > 
> > >
> http://busybox.net/cgi-bin/viewcvs.cgi/trunk/busybox/networking/udhcp/dh
> cpc.c?r2=16066&rev=16066&r1=16058
> > > 
> > > I would much rather undo r16066.
> > 
> > No.
> > 
> > May I ask you what will happen here:
> > 	memcpy(&server_addr, temp, 4);
> > on a big-endian 64-bit machine? Hint: server_addr is an 
> > _unsigned long_!
> > 
> > And no, I don't know who is that bright guy who used ulongs for
> > ipv4 addresses in udhcp code.
> > 
> > Okay, enough bitching for today. I will apply following patch to svn,
> > does it work for you?
> > --
> > vda
> > 
> > Index: dhcpc.c
> > ===================================================================
> > --- dhcpc.c     (revision 17356)
> > +++ dhcpc.c     (working copy)
> > @@ -23,7 +23,7 @@
> >   * which holds IPv4 address, and the struct is passed by value (!!)
> >   */
> >  static unsigned long requested_ip; /* = 0 */
> > -static unsigned long server_addr;
> > +static uint32_t server_addr;
> >  static unsigned long timeout;
> >  static int packet_num; /* = 0 */
> >  static int fd = -1;
> > @@ -413,7 +413,8 @@
> >                                 if (*message == DHCPOFFER) {
> >                                         temp = get_option(&packet,
> DHCP_SERVER_ID);
> >                                         if (temp) {
> > -                                               server_addr =
> *(uint32_t*)temp;
> > +                                               /* can be misaligned,
> thus memcpy */
> > +                                               memcpy(&server_addr,
> temp, 4);
> 
> Just a minor detail, but wouldn't it be better to use
> sizeof server_addr rather than 4?

Hmm, I'm pretty sure that ipv4 address is a 4 byte value.
It's other way around: sizeof server_addr _must_ be 4.
--
vda



More information about the busybox mailing list