| Message ID | 1528193509-7063-3-git-send-email-thuth@redhat.com |
|---|---|
| State | Accepted |
| Headers | show |
| Series | Clean-up and fix-up patches for pxelinux.cfg | expand |
On Tue, 5 Jun 2018 12:11:48 +0200 Thomas Huth <thuth@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. While we're at it, add a sanity check that > the function really did not load more bytes than requested. > > Signed-off-by: Thomas Huth <thuth@redhat.com> > --- Reviewed-by: Greg Kurz <groug@kaod.org> > lib/libnet/pxelinux.c | 14 ++++++++------ > 1 file changed, 8 insertions(+), 6 deletions(-) > > diff --git a/lib/libnet/pxelinux.c b/lib/libnet/pxelinux.c > index 718ceca..939a92c 100644 > --- a/lib/libnet/pxelinux.c > +++ b/lib/libnet/pxelinux.c > @@ -16,6 +16,7 @@ > > #include <stdio.h> > #include <string.h> > +#include <assert.h> > #include "tftp.h" > #include "pxelinux.h" > > @@ -95,7 +96,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 +105,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 +114,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 +128,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 +138,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 +241,10 @@ 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; > + assert(rc < cfgsize); > > return pxelinux_parse_cfg(cfgbuf, rc, entries, max_entries, def_ent); > }
diff --git a/lib/libnet/pxelinux.c b/lib/libnet/pxelinux.c index 718ceca..939a92c 100644 --- a/lib/libnet/pxelinux.c +++ b/lib/libnet/pxelinux.c @@ -16,6 +16,7 @@ #include <stdio.h> #include <string.h> +#include <assert.h> #include "tftp.h" #include "pxelinux.h" @@ -95,7 +96,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 +105,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 +114,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 +128,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 +138,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 +241,10 @@ 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; + assert(rc < cfgsize); return pxelinux_parse_cfg(cfgbuf, rc, entries, max_entries, def_ent); }
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. While we're at it, add a sanity check that the function really did not load more bytes than requested. Signed-off-by: Thomas Huth <thuth@redhat.com> --- lib/libnet/pxelinux.c | 14 ++++++++------ 1 file changed, 8 insertions(+), 6 deletions(-)