[Date Prev][Date Next] [Thread Prev][Thread Next] [Date Index] [Thread Index]

Bug#426756: ported pypxeboot patches to busybox udhcpc



12.05.2011 20:00, bm wrote:
> Hi,
> 
> the attached patches work with busybox-1.17.1 in squeeze and
> busybox-1.18.4 in unstable. They are based on the patch in
> http://www.grid.ie/pypxeboot/pypxeboot-0.0.3.tar.gz
> 
> Would be great if they could be included, so that we can have
> paravirtualized pxe bootable domUs in xen!

So basically, the only modification this patch provides is
to add -M option to specify alternative MAC address, right?

I think it should be addressed upstream first, or at least
tried to.  I may take care of that.

But I've at least one comment about the code, see below.

[]
> --- busybox-1.17.1-debianorig//networking/udhcp/dhcpc.c	2010-07-06 04:25:54.000000000 +0200
> +++ busybox-1.17.1-patched//networking/udhcp/dhcpc.c	2011-05-12 17:26:44.000000000 +0200
> @@ -766,7 +766,8 @@
>  int udhcpc_main(int argc UNUSED_PARAM, char **argv)
>  {
>  	uint8_t *temp, *message;
> -	const char *str_c, *str_V, *str_h, *str_F, *str_r;
> +	uint8_t hwmac[6];
> +	const char *str_c, *str_V, *str_h, *str_F, *str_r, *str_M;
>  	IF_FEATURE_UDHCP_PORT(char *str_P;)
[]
> +	if (opt & (OPT_M)) {
> +	        int nrmacfields = 0;
> +                nrmacfields=sscanf(str_M,"%x:%x:%x:%x:%x:%x",
> +                                        (unsigned int *)&hwmac[0],
> +                                        (unsigned int *)&hwmac[1],
> +                                        (unsigned int *)&hwmac[2],
> +                                        (unsigned int *)&hwmac[3],
> +                                        (unsigned int *)&hwmac[4],
> +                                        (unsigned int *)&hwmac[5]);
> +                if (nrmacfields == 6) memcpy(client_config.client_mac, hwmac, 6);
> +	}

This has 2 problems.  First, you can't cast uint8_t to unsigned int*,
the result is at least endian-dependent (where's the byte of int which
should go to our uint8 -- in the end of int or at the beginning of it),
and sscanf will access bytes which are not within the hwmac array.

Second, if you make a mistake/typo in the mac address, it will be
silently ignored - at least a warning should be printed in this
case.

/mjt



Reply to: