[SLOF] [PATCH 1/2] lib/libnet/pxelinux: Make the size handling for pxelinux_load_cfg more logical
Greg Kurz
groug at kaod.org
Fri Jun 1 17:24:17 AEST 2018
On Fri, 1 Jun 2018 07:26:50 +0200
Thomas Huth <thuth at redhat.com> wrote:
> The pxelinux_load_cfg() function always tried to load one byte less than
> its parameter said (so that we've got space for a terminating NUL-character
> later). This is not very intuitive, let's better ask for one byte less
> when we call the function.
It's better indeed.
> While we're at it, add a sanity check that
> the function really did not load more bytes than requested.
>
No sure for this... see below.
> Signed-off-by: Thomas Huth <thuth at redhat.com>
> ---
> lib/libnet/pxelinux.c | 16 ++++++++++------
> 1 file changed, 10 insertions(+), 6 deletions(-)
>
> diff --git a/lib/libnet/pxelinux.c b/lib/libnet/pxelinux.c
> index 718ceca..eaead48 100644
> --- a/lib/libnet/pxelinux.c
> +++ b/lib/libnet/pxelinux.c
> @@ -95,7 +95,7 @@ static int pxelinux_load_cfg(filename_ip_t *fn_ip, uint8_t *mac, const char *uui
> return -1;
> }
> strcpy(baseptr, fn_ip->pl_cfgfile);
> - rc = pxelinux_tftp_load(fn_ip, cfgbuf, cfgbufsize - 1, retries);
> + rc = pxelinux_tftp_load(fn_ip, cfgbuf, cfgbufsize, retries);
> if (rc > 0) {
> return rc;
> }
> @@ -104,7 +104,7 @@ static int pxelinux_load_cfg(filename_ip_t *fn_ip, uint8_t *mac, const char *uui
> /* Try to load config file with name based on the VM UUID */
> if (uuid) {
> strcpy(baseptr, uuid);
> - rc = pxelinux_tftp_load(fn_ip, cfgbuf, cfgbufsize - 1, retries);
> + rc = pxelinux_tftp_load(fn_ip, cfgbuf, cfgbufsize, retries);
> if (rc > 0) {
> return rc;
> }
> @@ -113,7 +113,7 @@ static int pxelinux_load_cfg(filename_ip_t *fn_ip, uint8_t *mac, const char *uui
> /* Look for config file with MAC address in its name */
> sprintf(baseptr, "01-%02x-%02x-%02x-%02x-%02x-%02x",
> mac[0], mac[1], mac[2], mac[3], mac[4], mac[5]);
> - rc = pxelinux_tftp_load(fn_ip, cfgbuf, cfgbufsize - 1, retries);
> + rc = pxelinux_tftp_load(fn_ip, cfgbuf, cfgbufsize, retries);
> if (rc > 0) {
> return rc;
> }
> @@ -127,7 +127,7 @@ static int pxelinux_load_cfg(filename_ip_t *fn_ip, uint8_t *mac, const char *uui
> fn_ip->own_ip & 0xff);
> for (idx = 0; idx <= 7; idx++) {
> baseptr[8 - idx] = 0;
> - rc = pxelinux_tftp_load(fn_ip, cfgbuf, cfgbufsize - 1,
> + rc = pxelinux_tftp_load(fn_ip, cfgbuf, cfgbufsize,
> retries);
> if (rc > 0) {
> return rc;
> @@ -137,7 +137,7 @@ static int pxelinux_load_cfg(filename_ip_t *fn_ip, uint8_t *mac, const char *uui
>
> /* Try "default" config file */
> strcpy(baseptr, "default");
> - rc = pxelinux_tftp_load(fn_ip, cfgbuf, cfgbufsize - 1, retries);
> + rc = pxelinux_tftp_load(fn_ip, cfgbuf, cfgbufsize, retries);
>
> return rc;
> }
> @@ -240,9 +240,13 @@ int pxelinux_load_parse_cfg(filename_ip_t *fn_ip, uint8_t *mac, const char *uuid
> {
> int rc;
>
> - rc = pxelinux_load_cfg(fn_ip, mac, uuid, retries, cfgbuf, cfgsize);
> + rc = pxelinux_load_cfg(fn_ip, mac, uuid, retries, cfgbuf, cfgsize - 1);
> if (rc < 0)
> return rc;
> + if (rc >= cfgsize) { /* This should never happen */
Can this even happen at all ? I see tftp() seem to have some code that would
cause pxelinux_load_cfg() to return -2 if the buffer length is reached... and
anyway, if it didn't then rc >= cfgsize would mean something was written past
the buffer or am I missing something... how can we cope with that ?
> + puts("Downloaded pxelinux.cfg is too big!");
> + return -1;
> + }
>
> return pxelinux_parse_cfg(cfgbuf, rc, entries, max_entries, def_ent);
> }
More information about the SLOF
mailing list