[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [PATCH v2 6/9] net: dhcp: use DHCP options for name and bootfile
From: |
Andre Przywara |
Subject: |
Re: [PATCH v2 6/9] net: dhcp: use DHCP options for name and bootfile |
Date: |
Thu, 7 Mar 2019 10:36:12 +0000 |
On Thu, 21 Feb 2019 19:49:08 +0100
Daniel Kiper <address@hidden> wrote:
> On Tue, Feb 12, 2019 at 05:46:57PM +0000, Andre Przywara wrote:
> > From: Andrei Borzenkov <address@hidden>
> >
> > The BOOTP RFC describes the boot file name and the server name as being
> > part of the integral BOOTP data structure, with some limits on the size
> > of them.
> > DHCP extends this by allowing them to be separate DHCP options, which is
> > more flexible.
> >
> > Teach the code dealing with those fields to check for those DHCP options
> > first and use this information, if provided. We fall back to using the
> > BOOTP information if those options are not used.
> >
> > Signed-off-by: Andre Przywara <address@hidden>
> > ---
> > grub-core/net/bootp.c | 72 +++++++++++++++++++++++++++++++------------
> > include/grub/net.h | 2 ++
> > 2 files changed, 54 insertions(+), 20 deletions(-)
> >
> > diff --git a/grub-core/net/bootp.c b/grub-core/net/bootp.c
> > index 42117b72d..0c6756ea0 100644
> > --- a/grub-core/net/bootp.c
> > +++ b/grub-core/net/bootp.c
> > @@ -169,7 +169,9 @@ grub_net_configure_by_dhcp_ack (const char *name,
> > int mask = -1;
> > char server_ip[sizeof ("xxx.xxx.xxx.xxx")];
> > const grub_uint8_t *opt;
> > - grub_uint8_t opt_len;
> > + grub_uint8_t opt_len, overload = 0;
> > + const char *boot_file = 0, *server_name = 0;
> > + grub_size_t boot_file_len, server_name_len;
> >
> > addr.type = GRUB_NET_NETWORK_LEVEL_PROTOCOL_IPV4;
> > addr.ipv4 = bp->your_ip;
> > @@ -188,9 +190,36 @@ grub_net_configure_by_dhcp_ack (const char *name,
> > if (!inter)
> > return 0;
> >
> > - if (size > OFFSET_OF (boot_file, bp))
> > - grub_env_set_net_property (name, "boot_file", bp->boot_file,
> > - sizeof (bp->boot_file));
> > + opt = find_dhcp_option (bp, size, GRUB_NET_DHCP_OVERLOAD, &opt_len);
> > + if (opt && opt_len == 1)
> > + overload = *opt;
> > +
> > + opt = find_dhcp_option (bp, size, GRUB_NET_DHCP_TFTP_SERVER_NAME,
> > &opt_len);
> > + if (opt && opt_len)
> > + {
> > + server_name = (const char *) opt;
> > + server_name_len = opt_len;
> > + }
> > + else if (size > OFFSET_OF (server_name, bp) && !(overload &
> > GRUB_DHCP_OPT_OVERLOAD_SNAME) &&
> > + bp->server_name[0])
> > + {
> > + server_name = bp->server_name;
> > + server_name_len = sizeof (bp->server_name);
> > + }
> > +
> > + opt = find_dhcp_option (bp, size, GRUB_NET_DHCP_BOOTFILE_NAME, &opt_len);
> > + if (opt && opt_len)
> > + {
> > + boot_file = (const char *) opt;
> > + boot_file_len = opt_len;
> > + }
> > + else if (size > OFFSET_OF (boot_file, bp) && !(overload &&
> > GRUB_DHCP_OPT_OVERLOAD_FILE) &&
> > + bp->boot_file[0])
> > + {
> > + boot_file = bp->boot_file;
> > + boot_file_len = sizeof (bp->boot_file);
> > + }
> > +
> > if (bp->server_ip)
> > {
> > grub_snprintf (server_ip, sizeof (server_ip), "%d.%d.%d.%d",
> > @@ -221,35 +250,38 @@ grub_net_configure_by_dhcp_ack (const char *name,
> > *device = grub_xasprintf ("tftp,%s", server_ip);
> > grub_print_error ();
> > }
> > - if (size > OFFSET_OF (server_name, bp)
> > - && bp->server_name[0])
> > +
> > + if (server_name)
> > {
> > - grub_env_set_net_property (name, "dhcp_server_name", bp->server_name,
> > - sizeof (bp->server_name));
> > + grub_env_set_net_property (name, "dhcp_server_name", server_name,
> > server_name_len);
> > if (is_def && !grub_net_default_server)
> > {
> > - grub_net_default_server = grub_strdup (bp->server_name);
> > + grub_net_default_server = grub_strdup (server_name);
> > grub_print_error ();
> > }
> > if (device && !*device)
> > {
> > - *device = grub_xasprintf ("tftp,%s", bp->server_name);
> > + *device = grub_xasprintf ("tftp,%s", server_name);
> > grub_print_error ();
> > }
> > }
> >
> > - if (size > OFFSET_OF (boot_file, bp) && path)
> > + if (boot_file)
> > {
> > - *path = grub_strndup (bp->boot_file, sizeof (bp->boot_file));
> > - grub_print_error ();
> > - if (*path)
> > + grub_env_set_net_property (name, "boot_file", boot_file,
> > boot_file_len);
> > + if (path)
> > {
> > - char *slash;
> > - slash = grub_strrchr (*path, '/');
> > - if (slash)
> > - *slash = 0;
> > - else
> > - **path = 0;
> > + *path = grub_strndup (boot_file, boot_file_len);
> > + grub_print_error ();
> > + if (*path)
> > + {
> > + char *slash;
> > + slash = grub_strrchr (*path, '/');
> > + if (slash)
> > + *slash = 0;
> > + else
> > + **path = 0;
> > + }
>
> I am not sure why you are changing the code starting from
> "if (size > OFFSET_OF (boot_file, bp) && path)". Could you
> enlighten me?
The diff is indeed somewhat confusing. So several things are changed here:
- The "boot file" could be specified via *two* ways now, as a DHCP option or
using the legacy BOOTP structure field. This is detected at separate places,
that's why we change the first line to check for a valid boot_file pointer,
instead of hardcoding the BOOTP structure field way here.
- grub_env_set_net_property() was called before when detecting the BOOTP
structure field (see above), we do it now here, to cover both ways.
- There is an additional molly guard to check that "path" is not a NULL pointer.
I am attaching a slightly modified wide diff (diff -yb) to make this clearer.
Cheers,
Andre.
boot_file.diff
Description: Text Data
[Prev in Thread] |
Current Thread |
[Next in Thread] |
- Re: [PATCH v2 6/9] net: dhcp: use DHCP options for name and bootfile,
Andre Przywara <=