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