Messages in this thread | | | From | Chris Packham <> | Subject | Re: [PATCH] tipc: Avoid copying bytes beyond the supplied data | Date | Sun, 5 May 2019 20:20:14 +0000 |
| |
On 4/05/19 4:45 PM, David Miller wrote: > From: Chris Packham <chris.packham@alliedtelesis.co.nz> > Date: Thu, 2 May 2019 15:10:04 +1200 > >> TLV_SET is called with a data pointer and a len parameter that tells us >> how many bytes are pointed to by data. When invoking memcpy() we need >> to careful to only copy len bytes. >> >> Previously we would copy TLV_LENGTH(len) bytes which would copy an extra >> 4 bytes past the end of the data pointer which newer GCC versions >> complain about. >> >> In file included from test.c:17: >> In function 'TLV_SET', >> inlined from 'test' at test.c:186:5: >> /usr/include/linux/tipc_config.h:317:3: >> warning: 'memcpy' forming offset [33, 36] is out of the bounds [0, 32] >> of object 'bearer_name' with type 'char[32]' [-Warray-bounds] >> memcpy(TLV_DATA(tlv_ptr), data, tlv_len); >> ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ >> test.c: In function 'test': >> test.c::161:10: note: >> 'bearer_name' declared here >> char bearer_name[TIPC_MAX_BEARER_NAME]; >> ^~~~~~~~~~~ >> >> Signed-off-by: Chris Packham <chris.packham@alliedtelesis.co.nz> > > But now the pad bytes at the end are uninitialized. > > The whole idea is that the encapsulating TLV object has to be rounded > up in size based upon the given 'len' for the data. >
TLV_LENGTH() does not account for any padding bytes due to the alignment. TLV_SPACE() does but that wasn't used in the code before my change.
Are you suggesting something like this
- if (len && data) - memcpy(TLV_DATA(tlv_ptr), data, tlv_len); + if (len && data) { + memcpy(TLV_DATA(tlv_ptr), data, len); + memset(TLV_DATA(tlv_ptr) + len, 0, TLV_SPACE(len) - TLV_LENGTH(len)); + }
| |