lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening PHC | |
Open Source and information security mailing list archives
| ||
|
Date: Wed, 7 Apr 2021 11:56:06 -0700 From: Kees Cook <keescook@...omium.org> To: "Gustavo A. R. Silva" <gustavoars@...nel.org> Cc: linux-kernel@...r.kernel.org, Kalle Valo <kvalo@...eaurora.org>, "David S. Miller" <davem@...emloft.net>, Jakub Kicinski <kuba@...nel.org>, linux-wireless@...r.kernel.org, netdev@...r.kernel.org, linux-hardening@...r.kernel.org Subject: Re: [PATCH v2 1/2][next] wl3501_cs: Fix out-of-bounds warning in wl3501_send_pkt On Wed, Mar 31, 2021 at 04:44:29PM -0500, Gustavo A. R. Silva wrote: > Fix the following out-of-bounds warning by enclosing > structure members daddr and saddr into new struct addr: > > arch/x86/include/asm/string_32.h:182:25: warning: '__builtin_memcpy' offset [18, 23] from the object at 'sig' is out of the bounds of referenced subobject 'daddr' with type 'u8[6]' {aka 'unsigned char[6]'} at offset 11 [-Warray-bounds] > > Refactor the code, accordingly: > > $ pahole -C wl3501_md_req drivers/net/wireless/wl3501_cs.o > struct wl3501_md_req { > u16 next_blk; /* 0 2 */ > u8 sig_id; /* 2 1 */ > u8 routing; /* 3 1 */ > u16 data; /* 4 2 */ > u16 size; /* 6 2 */ > u8 pri; /* 8 1 */ > u8 service_class; /* 9 1 */ > struct { > u8 daddr[6]; /* 10 6 */ > u8 saddr[6]; /* 16 6 */ > } addr; /* 10 12 */ > > /* size: 22, cachelines: 1, members: 8 */ > /* last cacheline: 22 bytes */ > }; > > The problem is that the original code is trying to copy data into a > couple of arrays adjacent to each other in a single call to memcpy(). > Now that a new struct _addr_ enclosing those two adjacent arrays > is introduced, memcpy() doesn't overrun the length of &sig.daddr[0], > because the address of the new struct object _addr_ is used as > destination, instead. > > Also, this helps with the ongoing efforts to enable -Warray-bounds and > avoid confusing the compiler. > > Link: https://github.com/KSPP/linux/issues/109 > Reported-by: kernel test robot <lkp@...el.com> > Build-tested-by: kernel test robot <lkp@...el.com> > Link: https://lore.kernel.org/lkml/60641d9b.2eNLedOGSdcSoAV2%25lkp@intel.com/ > Signed-off-by: Gustavo A. R. Silva <gustavoars@...nel.org> Thanks, this makes the code much easier for the compiler to validate at compile time. These cross-field memcpy()s are weird. I like the solution here. Reviewed-by: Kees Cook <keescook@...omium.org> -- Kees Cook
Powered by blists - more mailing lists