Messages in this thread | | | Date | Sun, 5 May 2019 16:56:58 -0400 | From | Arvind Sankar <> | Subject | Re: [PATCH] tipc: Avoid copying bytes beyond the supplied data |
| |
On Sun, May 05, 2019 at 08:20:14PM +0000, Chris Packham wrote: > 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)); > + } > >
For zeroing out the padding, should that be done in TCM_SET in the same file as well? That one only copies data_len bytes but doesn't zero out any alignment padding.
| |