Hi,
In preparation for FORTIFY_SOURCE performing compile-time and run-time
field bounds checking for memcpy(), memmove(), and memset(), avoid
intentionally writing across neighboring fields.
These three changes have been living in my memcpy() series[1], but have
no external dependencies. It's probably better to have these go via
netdev.
Thanks!
-Kees
[1] https://lore.kernel.org/lkml/20210818060533.3569517-1-keescook@chromium.org/
Kees Cook (3):
ipw2x00: Avoid field-overflowing memcpy()
net/mlx5e: Avoid field-overflowing memcpy()
pcmcia: ray_cs: Split memcpy() to avoid bounds check warning
drivers/net/ethernet/mellanox/mlx5/core/en.h | 4 +-
.../net/ethernet/mellanox/mlx5/core/en/xdp.c | 4 +-
.../net/wireless/intel/ipw2x00/libipw_rx.c | 56 ++++++-------------
drivers/net/wireless/ray_cs.c | 4 +-
4 files changed, 25 insertions(+), 43 deletions(-)
--
2.30.2
In preparation for FORTIFY_SOURCE performing compile-time and run-time
field bounds checking for memcpy(), memmove(), and memset(), avoid
intentionally writing across neighboring fields.
libipw_read_qos_param_element() copies a struct libipw_info_element
into a struct libipw_qos_information_element, but is actually wanting to
copy into the larger struct libipw_qos_parameter_info (the contents of
ac_params_record[] is later examined). Refactor the routine to perform
centralized checks, and copy the entire contents directly (since the id
and len members match the elementID and length members):
struct libipw_info_element {
u8 id;
u8 len;
u8 data[];
} __packed;
struct libipw_qos_information_element {
u8 elementID;
u8 length;
u8 qui[QOS_OUI_LEN];
u8 qui_type;
u8 qui_subtype;
u8 version;
u8 ac_info;
} __packed;
struct libipw_qos_parameter_info {
struct libipw_qos_information_element info_element;
u8 reserved;
struct libipw_qos_ac_parameter ac_params_record[QOS_QUEUE_NUM];
} __packed;
Cc: Stanislav Yakovlev <stas.yakovlev@gmail.com>
Cc: Kalle Valo <redacted>
Cc: "David S. Miller" <davem@davemloft.net>
Cc: Jakub Kicinski <kuba@kernel.org>
Cc: linux-wireless@vger.kernel.org
Cc: netdev@vger.kernel.org
Signed-off-by: Kees Cook <redacted>
---
.../net/wireless/intel/ipw2x00/libipw_rx.c | 56 ++++++-------------
1 file changed, 17 insertions(+), 39 deletions(-)
In preparation for FORTIFY_SOURCE performing compile-time and run-time
field bounds checking for memcpy(), memmove(), and memset(), avoid
intentionally writing across neighboring fields.
Split memcpy() for each address range to help memcpy() correctly reason
about the bounds checking. Avoids the future warning:
In function 'fortify_memcpy_chk',
inlined from 'memcpy_toio' at ./include/asm-generic/io.h:1204:2,
inlined from 'ray_build_header.constprop' at drivers/net/wireless/ray_cs.c:984:3:
./include/linux/fortify-string.h:285:4: warning: call to '__write_overflow_field' declared with attribute warning: detected write beyond size of field (1st parameter); maybe use struct_group()? [-Wattribute-warning]
285 | __write_overflow_field(p_size_field, size);
| ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
Cc: Kalle Valo <redacted>
Cc: "David S. Miller" <davem@davemloft.net>
Cc: Jakub Kicinski <kuba@kernel.org>
Cc: linux-wireless@vger.kernel.org
Cc: netdev@vger.kernel.org
Signed-off-by: Kees Cook <redacted>
---
drivers/net/wireless/ray_cs.c | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
@@ -982,7 +982,9 @@ AP to AP 1 1 dest AP src AP dest sourceif(local->net_type==ADHOC){writeb(0,&ptx->mac.frame_ctl_2);memcpy_toio(ptx->mac.addr_1,((structethhdr*)data)->h_dest,-2*ADDRLEN);+ADDRLEN);+memcpy_toio(ptx->mac.addr_2,((structethhdr*)data)->h_source,+ADDRLEN);memcpy_toio(ptx->mac.addr_3,local->bss_id,ADDRLEN);}else{/* infrastructure */
From: Jakub Kicinski <kuba@kernel.org> Date: 2021-08-20 17:02:23
On Thu, 19 Aug 2021 13:28:22 -0700 Kees Cook wrote:
Hi,
In preparation for FORTIFY_SOURCE performing compile-time and run-time
field bounds checking for memcpy(), memmove(), and memset(), avoid
intentionally writing across neighboring fields.
These three changes have been living in my memcpy() series[1], but have
no external dependencies. It's probably better to have these go via
netdev.
Thanks.
Kalle, Saeed - would you like to take the relevant changes? Presumably
they would get into net-next anyway by the time the merge window opens.
From: Kalle Valo <hidden> Date: 2021-08-21 17:16:10
Kees Cook [off-list ref] wrote:
In preparation for FORTIFY_SOURCE performing compile-time and run-time
field bounds checking for memcpy(), memmove(), and memset(), avoid
intentionally writing across neighboring fields.
libipw_read_qos_param_element() copies a struct libipw_info_element
into a struct libipw_qos_information_element, but is actually wanting to
copy into the larger struct libipw_qos_parameter_info (the contents of
ac_params_record[] is later examined). Refactor the routine to perform
centralized checks, and copy the entire contents directly (since the id
and len members match the elementID and length members):
struct libipw_info_element {
u8 id;
u8 len;
u8 data[];
} __packed;
struct libipw_qos_information_element {
u8 elementID;
u8 length;
u8 qui[QOS_OUI_LEN];
u8 qui_type;
u8 qui_subtype;
u8 version;
u8 ac_info;
} __packed;
struct libipw_qos_parameter_info {
struct libipw_qos_information_element info_element;
u8 reserved;
struct libipw_qos_ac_parameter ac_params_record[QOS_QUEUE_NUM];
} __packed;
Cc: Stanislav Yakovlev <stas.yakovlev@gmail.com>
Cc: Kalle Valo <redacted>
Cc: "David S. Miller" <davem@davemloft.net>
Cc: Jakub Kicinski <kuba@kernel.org>
Cc: linux-wireless@vger.kernel.org
Cc: netdev@vger.kernel.org
Signed-off-by: Kees Cook <redacted>