| Message ID | 20200113112258.165490-1-colin.king@canonical.com |
|---|---|
| State | Accepted |
| Headers | show |
| Series | lib: fwts_multiproc: add sanity check to data->size to clean up static analysis warning | expand |
On 2020-01-13 4:22 a.m., Colin King wrote: > From: Colin Ian King <colin.king@canonical.com> > > Static analysis warns that the data->size is not validated so add a simple > sanity check on the size. This is a moot point as we know the size is never > going to be huge and we have to trust the given table size, but this at least > sanity checks things and keeps static analyzers happy. > > Addresses-Coverity: ("Untrusted value as argument") > Signed-off-by: Colin Ian King <colin.king@canonical.com> > --- > src/lib/src/fwts_multiproc.c | 7 +++++++ > 1 file changed, 7 insertions(+) > > diff --git a/src/lib/src/fwts_multiproc.c b/src/lib/src/fwts_multiproc.c > index 1402c1a7..c79c0be3 100644 > --- a/src/lib/src/fwts_multiproc.c > +++ b/src/lib/src/fwts_multiproc.c > @@ -39,6 +39,8 @@ > #define BIOS_START (0x000e0000) /* Start of BIOS memory */ > #define BIOS_END (0x000fffff) /* End of BIOS memory */ > > +#define MAX_SIZE(x) (1UL << (sizeof(x) * 8)) > + > /* > * fwts_mp_get_address() > * scan for _MP_ floating pointer, set phys_addr if found. > @@ -111,6 +113,7 @@ int fwts_mp_data_get(fwts_mp_data *data) > void *mem; > uint8_t *tmp; > fwts_mp_config_table_header *header; > + int max_data_size; > > if (data == NULL) > return FWTS_ERROR; > @@ -130,6 +133,10 @@ int fwts_mp_data_get(fwts_mp_data *data) > > data->size = header->base_table_length + > ((header->spec_rev == 1) ? 0 : header->extended_table_length); > + max_data_size = MAX_SIZE(header->base_table_length) + > + MAX_SIZE(header->extended_table_length); > + if (data->size < 0 || data->size > max_data_size) > + return FWTS_ERROR; > > /* Remap with full header and table now we know how big it is */ > (void)fwts_munmap(mem, sizeof(fwts_mp_config_table_header)); > Acked-by: Alex Hung <alex.hung@canonical.com>
On 1/13/20 7:22 PM, Colin King wrote: > From: Colin Ian King <colin.king@canonical.com> > > Static analysis warns that the data->size is not validated so add a simple > sanity check on the size. This is a moot point as we know the size is never > going to be huge and we have to trust the given table size, but this at least > sanity checks things and keeps static analyzers happy. > > Addresses-Coverity: ("Untrusted value as argument") > Signed-off-by: Colin Ian King <colin.king@canonical.com> > --- > src/lib/src/fwts_multiproc.c | 7 +++++++ > 1 file changed, 7 insertions(+) > > diff --git a/src/lib/src/fwts_multiproc.c b/src/lib/src/fwts_multiproc.c > index 1402c1a7..c79c0be3 100644 > --- a/src/lib/src/fwts_multiproc.c > +++ b/src/lib/src/fwts_multiproc.c > @@ -39,6 +39,8 @@ > #define BIOS_START (0x000e0000) /* Start of BIOS memory */ > #define BIOS_END (0x000fffff) /* End of BIOS memory */ > > +#define MAX_SIZE(x) (1UL << (sizeof(x) * 8)) > + > /* > * fwts_mp_get_address() > * scan for _MP_ floating pointer, set phys_addr if found. > @@ -111,6 +113,7 @@ int fwts_mp_data_get(fwts_mp_data *data) > void *mem; > uint8_t *tmp; > fwts_mp_config_table_header *header; > + int max_data_size; > > if (data == NULL) > return FWTS_ERROR; > @@ -130,6 +133,10 @@ int fwts_mp_data_get(fwts_mp_data *data) > > data->size = header->base_table_length + > ((header->spec_rev == 1) ? 0 : header->extended_table_length); > + max_data_size = MAX_SIZE(header->base_table_length) + > + MAX_SIZE(header->extended_table_length); > + if (data->size < 0 || data->size > max_data_size) > + return FWTS_ERROR; > > /* Remap with full header and table now we know how big it is */ > (void)fwts_munmap(mem, sizeof(fwts_mp_config_table_header)); > Acked-by: Ivan Hu <ivan.hu@canonical.com>
diff --git a/src/lib/src/fwts_multiproc.c b/src/lib/src/fwts_multiproc.c index 1402c1a7..c79c0be3 100644 --- a/src/lib/src/fwts_multiproc.c +++ b/src/lib/src/fwts_multiproc.c @@ -39,6 +39,8 @@ #define BIOS_START (0x000e0000) /* Start of BIOS memory */ #define BIOS_END (0x000fffff) /* End of BIOS memory */ +#define MAX_SIZE(x) (1UL << (sizeof(x) * 8)) + /* * fwts_mp_get_address() * scan for _MP_ floating pointer, set phys_addr if found. @@ -111,6 +113,7 @@ int fwts_mp_data_get(fwts_mp_data *data) void *mem; uint8_t *tmp; fwts_mp_config_table_header *header; + int max_data_size; if (data == NULL) return FWTS_ERROR; @@ -130,6 +133,10 @@ int fwts_mp_data_get(fwts_mp_data *data) data->size = header->base_table_length + ((header->spec_rev == 1) ? 0 : header->extended_table_length); + max_data_size = MAX_SIZE(header->base_table_length) + + MAX_SIZE(header->extended_table_length); + if (data->size < 0 || data->size > max_data_size) + return FWTS_ERROR; /* Remap with full header and table now we know how big it is */ (void)fwts_munmap(mem, sizeof(fwts_mp_config_table_header));