[U-Boot] [PATCH v2] net: Do not overwrite options found in overloaded 'file' field
Joe Hershberger
joe.hershberger at gmail.com
Thu Oct 29 19:51:13 CET 2015
Hi Stefan,
On Wed, Oct 28, 2015 at 4:05 PM, Joe Hershberger
<joe.hershberger at gmail.com> wrote:
> Hi Stefan,
>
> On Thu, Sep 3, 2015 at 5:31 PM, Stefan Brüns
> <stefan.bruens at rwth-aachen.de> wrote:
>> If 'file' is overloaded, it is wrong to get or put the bootfile name
>> from it/to it.
>>
>> Signed-off-by: Stefan Brüns <stefan.bruens at rwth-aachen.de>
>> Acked-by: Joe Hershberger <joe.hershberger at ni.com>
>> ---
>>
>> v2: added missing break before empty "case 53:", no functional change
>>
>> net/bootp.c | 41 +++++++++++++++--------------------------
>> 1 file changed, 15 insertions(+), 26 deletions(-)
>>
>> diff --git a/net/bootp.c b/net/bootp.c
>> index 7fd29ee..72a956d 100644
>> --- a/net/bootp.c
>> +++ b/net/bootp.c
>> @@ -64,6 +64,9 @@ char net_root_path[64] = {0,}; /* Our bootpath */
>> static dhcp_state_t dhcp_state = INIT;
>> static u32 dhcp_leasetime;
>> static struct in_addr dhcp_server_ip;
>> +static u8 dhcp_option_overload;
>> +#define OVERLOAD_FILE 1
>> +#define OVERLOAD_SNAME 2
>> static void dhcp_handler(uchar *pkt, unsigned dest, struct in_addr sip,
>> unsigned src, unsigned len);
>>
>> @@ -146,9 +149,11 @@ static void store_net_params(struct bootp_hdr *bp)
>> net_copy_ip(&net_server_ip, &bp->bp_siaddr);
>> memcpy(net_server_ethaddr,
>> ((struct ethernet_hdr *)net_rx_packet)->et_src, 6);
>> - if (strlen(bp->bp_file) > 0)
>> + if (!(dhcp_option_overload & OVERLOAD_FILE) &&
>> + (strlen(bp->bp_file) > 0)) {
>
> It seems that this fails to compile if CONFIG_CMD_DHCP is not defined
> (since the variables only exist inside that guard.
>
> I've fixed it up in place like this:
>
> @@ -146,9 +149,14 @@ static void store_net_params(struct bootp_hdr *bp)
> net_copy_ip(&net_server_ip, &bp->bp_siaddr);
> memcpy(net_server_ethaddr,
> ((struct ethernet_hdr *)net_rx_packet)->et_src, 6);
> - if (strlen(bp->bp_file) > 0)
> + if (
> +#if defined(CONFIG_CMD_DHCP)
> + !(dhcp_option_overload & OVERLOAD_FILE) &&
> +#endif
> + (strlen(bp->bp_file) > 0)) {
> copy_filename(net_boot_file_name, bp->bp_file,
> sizeof(net_boot_file_name));
> + }
>
> debug("net_boot_file_name: %s\n", net_boot_file_name);
>
> ------
>
> Let me know if that is not how you would like it fixed.
I'm moving forward with this. We can always apply another patch later if needed.
>> copy_filename(net_boot_file_name, bp->bp_file,
>> sizeof(net_boot_file_name));
>> + }
>>
>> debug("net_boot_file_name: %s\n", net_boot_file_name);
>>
>> @@ -770,6 +775,7 @@ static void dhcp_process_options(uchar *popt, struct bootp_hdr *bp)
>> #if defined(CONFIG_CMD_SNTP) && defined(CONFIG_BOOTP_TIMEOFFSET)
>> int *to_ptr;
>> #endif
>> + dhcp_option_overload = 0;
>>
>> while (popt < end && *popt != 0xff) {
>> oplen = *(popt + 1);
>> @@ -821,6 +827,9 @@ static void dhcp_process_options(uchar *popt, struct bootp_hdr *bp)
>> case 51:
>> net_copy_u32(&dhcp_leasetime, (u32 *)(popt + 2));
>> break;
>> + case 52:
>> + dhcp_option_overload = popt[2];
>> + break;
>> case 53: /* Ignore Message Type Option */
>> break;
>> case 54:
>> @@ -832,31 +841,11 @@ static void dhcp_process_options(uchar *popt, struct bootp_hdr *bp)
>> break;
>> case 66: /* Ignore TFTP server name */
>> break;
>> - case 67: /* vendor opt bootfile */
>> - /*
>> - * I can't use dhcp_vendorex_proc here because I need
>> - * to write into the bootp packet - even then I had to
>> - * pass the bootp packet pointer into here as the
>> - * second arg
>> - */
>> - size = truncate_sz("Opt Boot File",
>> - sizeof(bp->bp_file),
>> - oplen);
>> - if (bp->bp_file[0] == '\0' && size > 0) {
>> - /*
>> - * only use vendor boot file if we didn't
>> - * receive a boot file in the main non-vendor
>> - * part of the packet - god only knows why
>> - * some vendors chose not to use this perfectly
>> - * good spot to store the boot file (join on
>> - * Tru64 Unix) it seems mind bogglingly crazy
>> - * to me
>> - */
>> - printf("*** WARNING: using vendor "
>> - "optional boot file\n");
>> - memcpy(bp->bp_file, popt + 2, size);
>> - bp->bp_file[size] = '\0';
>> - }
>> + case 67: /* Bootfile option */
>> + size = truncate_sz("Bootfile",
>> + sizeof(net_boot_file_name), oplen);
>> + memcpy(&net_boot_file_name, popt + 2, size);
>> + net_boot_file_name[size] = 0;
>> break;
>> default:
>> #if defined(CONFIG_BOOTP_VENDOREX)
>> --
>> 2.1.4
>>
>> _______________________________________________
>> U-Boot mailing list
>> U-Boot at lists.denx.de
>> http://lists.denx.de/mailman/listinfo/u-boot
More information about the U-Boot
mailing list