| Message ID | 1ec62c2c-869d-9acf-138d-99baeaca07b0@redhat.com |
|---|---|
| State | Changes Requested |
| Delegated to: | Pablo Neira |
| Headers | show |
| Series | None | expand |
Hi Maya, On Thu, Feb 18, 2021 at 01:06:38PM +0200, Maya Rashish wrote: > When data is smaller than the destination, &ctr->pkts. > > This might introduce some issues since we're now not > filling the rest of the memory, but filling out with > uninitialized garbage is probably as bad as leaving it > as garbage. Probably you could update src/expr/ to use nftnl_assert_validate() to sanity check the input data length? Please, have a look at nftnl_assert_attr_exists() and nftnl_assert_validate(). > Signed-off-by: Maya Rashish <mrashish@redhat.com> > --- > include/utils.h | 2 ++ > src/expr/counter.c | 4 ++-- > 2 files changed, 4 insertions(+), 2 deletions(-) > > diff --git a/include/utils.h b/include/utils.h > index 8af5a8e..6b22e46 100644 > --- a/include/utils.h > +++ b/include/utils.h > @@ -67,6 +67,8 @@ void __nftnl_assert_attr_exists(uint16_t attr, uint16_t attr_max, > > #define array_size(arr) (sizeof(arr) / sizeof((arr)[0]) + __must_be_array(arr)) > > +#define MIN(a,b) ((a) > (b) ? (b) : (a)) > + > const char *nftnl_family2str(uint32_t family); > int nftnl_str2family(const char *family); > > diff --git a/src/expr/counter.c b/src/expr/counter.c > index 89a602e..fb036dd 100644 > --- a/src/expr/counter.c > +++ b/src/expr/counter.c > @@ -35,10 +35,10 @@ nftnl_expr_counter_set(struct nftnl_expr *e, uint16_t type, > > switch(type) { > case NFTNL_EXPR_CTR_BYTES: > - memcpy(&ctr->bytes, data, sizeof(ctr->bytes)); > + memcpy(&ctr->bytes, data, MIN(data_len, sizeof(ctr->bytes))); > break; > case NFTNL_EXPR_CTR_PACKETS: > - memcpy(&ctr->pkts, data, sizeof(ctr->pkts)); > + memcpy(&ctr->pkts, data, MIN(data_len, sizeof(ctr->pkts))); > break; > default: > return -1; > -- > 2.29.2 >
diff --git a/include/utils.h b/include/utils.h index 8af5a8e..6b22e46 100644 --- a/include/utils.h +++ b/include/utils.h @@ -67,6 +67,8 @@ void __nftnl_assert_attr_exists(uint16_t attr, uint16_t attr_max, #define array_size(arr) (sizeof(arr) / sizeof((arr)[0]) + __must_be_array(arr)) +#define MIN(a,b) ((a) > (b) ? (b) : (a)) + const char *nftnl_family2str(uint32_t family); int nftnl_str2family(const char *family); diff --git a/src/expr/counter.c b/src/expr/counter.c index 89a602e..fb036dd 100644 --- a/src/expr/counter.c +++ b/src/expr/counter.c @@ -35,10 +35,10 @@ nftnl_expr_counter_set(struct nftnl_expr *e, uint16_t type, switch(type) { case NFTNL_EXPR_CTR_BYTES: - memcpy(&ctr->bytes, data, sizeof(ctr->bytes)); + memcpy(&ctr->bytes, data, MIN(data_len, sizeof(ctr->bytes))); break; case NFTNL_EXPR_CTR_PACKETS: - memcpy(&ctr->pkts, data, sizeof(ctr->pkts)); + memcpy(&ctr->pkts, data, MIN(data_len, sizeof(ctr->pkts))); break; default: return -1;
When data is smaller than the destination, &ctr->pkts. This might introduce some issues since we're now not filling the rest of the memory, but filling out with uninitialized garbage is probably as bad as leaving it as garbage. Signed-off-by: Maya Rashish <mrashish@redhat.com> --- include/utils.h | 2 ++ src/expr/counter.c | 4 ++-- 2 files changed, 4 insertions(+), 2 deletions(-)