* [PATCH 0/3] net: Cleanups for FORTIFY_SOURCE
@ 2021-08-19 20:28 Kees Cook
2021-08-19 20:28 ` [PATCH 1/3] ipw2x00: Avoid field-overflowing memcpy() Kees Cook
` (3 more replies)
0 siblings, 4 replies; 9+ messages in thread
From: Kees Cook @ 2021-08-19 20:28 UTC (permalink / raw)
To: netdev
Cc: Kees Cook, Stanislav Yakovlev, Kalle Valo, David S. Miller,
Jakub Kicinski, Saeed Mahameed, Leon Romanovsky,
Alexei Starovoitov, Daniel Borkmann, Jesper Dangaard Brouer,
John Fastabend, Andrii Nakryiko, Martin KaFai Lau, Song Liu,
Yonghong Song, KP Singh, linux-kernel, linux-wireless,
linux-rdma, bpf, linux-hardening
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
^ permalink raw reply [flat|nested] 9+ messages in thread
* [PATCH 1/3] ipw2x00: Avoid field-overflowing memcpy()
2021-08-19 20:28 [PATCH 0/3] net: Cleanups for FORTIFY_SOURCE Kees Cook
@ 2021-08-19 20:28 ` Kees Cook
2021-08-21 17:15 ` Kalle Valo
2021-08-19 20:28 ` [PATCH 2/3] net/mlx5e: " Kees Cook
` (2 subsequent siblings)
3 siblings, 1 reply; 9+ messages in thread
From: Kees Cook @ 2021-08-19 20:28 UTC (permalink / raw)
To: netdev
Cc: Kees Cook, Stanislav Yakovlev, Kalle Valo, David S. Miller,
Jakub Kicinski, linux-wireless, Saeed Mahameed, Leon Romanovsky,
Alexei Starovoitov, Daniel Borkmann, Jesper Dangaard Brouer,
John Fastabend, Andrii Nakryiko, Martin KaFai Lau, Song Liu,
Yonghong Song, KP Singh, linux-kernel, linux-rdma, bpf,
linux-hardening
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 <kvalo@codeaurora.org>
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 <keescook@chromium.org>
---
.../net/wireless/intel/ipw2x00/libipw_rx.c | 56 ++++++-------------
1 file changed, 17 insertions(+), 39 deletions(-)
diff --git a/drivers/net/wireless/intel/ipw2x00/libipw_rx.c b/drivers/net/wireless/intel/ipw2x00/libipw_rx.c
index 5a2a723e480b..7a684b76f39b 100644
--- a/drivers/net/wireless/intel/ipw2x00/libipw_rx.c
+++ b/drivers/net/wireless/intel/ipw2x00/libipw_rx.c
@@ -927,7 +927,8 @@ static u8 qos_oui[QOS_OUI_LEN] = { 0x00, 0x50, 0xF2 };
static int libipw_verify_qos_info(struct libipw_qos_information_element
*info_element, int sub_type)
{
-
+ if (info_element->elementID != QOS_ELEMENT_ID)
+ return -1;
if (info_element->qui_subtype != sub_type)
return -1;
if (memcmp(info_element->qui, qos_oui, QOS_OUI_LEN))
@@ -943,57 +944,34 @@ static int libipw_verify_qos_info(struct libipw_qos_information_element
/*
* Parse a QoS parameter element
*/
-static int libipw_read_qos_param_element(struct libipw_qos_parameter_info
- *element_param, struct libipw_info_element
- *info_element)
+static int libipw_read_qos_param_element(
+ struct libipw_qos_parameter_info *element_param,
+ struct libipw_info_element *info_element)
{
- int ret = 0;
- u16 size = sizeof(struct libipw_qos_parameter_info) - 2;
+ size_t size = sizeof(*element_param);
- if ((info_element == NULL) || (element_param == NULL))
+ if (!element_param || !info_element || info_element->len != size - 2)
return -1;
- if (info_element->id == QOS_ELEMENT_ID && info_element->len == size) {
- memcpy(element_param->info_element.qui, info_element->data,
- info_element->len);
- element_param->info_element.elementID = info_element->id;
- element_param->info_element.length = info_element->len;
- } else
- ret = -1;
- if (ret == 0)
- ret = libipw_verify_qos_info(&element_param->info_element,
- QOS_OUI_PARAM_SUB_TYPE);
- return ret;
+ memcpy(element_param, info_element, size);
+ return libipw_verify_qos_info(&element_param->info_element,
+ QOS_OUI_PARAM_SUB_TYPE);
}
/*
* Parse a QoS information element
*/
-static int libipw_read_qos_info_element(struct
- libipw_qos_information_element
- *element_info, struct libipw_info_element
- *info_element)
+static int libipw_read_qos_info_element(
+ struct libipw_qos_information_element *element_info,
+ struct libipw_info_element *info_element)
{
- int ret = 0;
- u16 size = sizeof(struct libipw_qos_information_element) - 2;
+ size_t size = sizeof(struct libipw_qos_information_element) - 2;
- if (element_info == NULL)
+ if (!element_info || !info_element || info_element->len != size - 2)
return -1;
- if (info_element == NULL)
- return -1;
-
- if ((info_element->id == QOS_ELEMENT_ID) && (info_element->len == size)) {
- memcpy(element_info->qui, info_element->data,
- info_element->len);
- element_info->elementID = info_element->id;
- element_info->length = info_element->len;
- } else
- ret = -1;
- if (ret == 0)
- ret = libipw_verify_qos_info(element_info,
- QOS_OUI_INFO_SUB_TYPE);
- return ret;
+ memcpy(element_info, info_element, size);
+ return libipw_verify_qos_info(element_info, QOS_OUI_INFO_SUB_TYPE);
}
/*
--
2.30.2
^ permalink raw reply related [flat|nested] 9+ messages in thread
* [PATCH 2/3] net/mlx5e: Avoid field-overflowing memcpy()
2021-08-19 20:28 [PATCH 0/3] net: Cleanups for FORTIFY_SOURCE Kees Cook
2021-08-19 20:28 ` [PATCH 1/3] ipw2x00: Avoid field-overflowing memcpy() Kees Cook
@ 2021-08-19 20:28 ` Kees Cook
2021-08-19 20:28 ` [PATCH 3/3] pcmcia: ray_cs: Split memcpy() to avoid bounds check warning Kees Cook
2021-08-20 17:01 ` [PATCH 0/3] net: Cleanups for FORTIFY_SOURCE Jakub Kicinski
3 siblings, 0 replies; 9+ messages in thread
From: Kees Cook @ 2021-08-19 20:28 UTC (permalink / raw)
To: netdev
Cc: Kees Cook, Saeed Mahameed, Leon Romanovsky, David S. Miller,
Jakub Kicinski, Alexei Starovoitov, Daniel Borkmann,
Jesper Dangaard Brouer, John Fastabend, linux-rdma, bpf,
Stanislav Yakovlev, Kalle Valo, Andrii Nakryiko,
Martin KaFai Lau, Song Liu, Yonghong Song, KP Singh,
linux-kernel, linux-wireless, linux-hardening
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.
Use flexible arrays instead of zero-element arrays (which look like they
are always overflowing) and split the cross-field memcpy() into two halves
that can be appropriately bounds-checked by the compiler.
We were doing:
#define ETH_HLEN 14
#define VLAN_HLEN 4
...
#define MLX5E_XDP_MIN_INLINE (ETH_HLEN + VLAN_HLEN)
...
struct mlx5e_tx_wqe *wqe = mlx5_wq_cyc_get_wqe(wq, pi);
...
struct mlx5_wqe_eth_seg *eseg = &wqe->eth;
struct mlx5_wqe_data_seg *dseg = wqe->data;
...
memcpy(eseg->inline_hdr.start, xdptxd->data, MLX5E_XDP_MIN_INLINE);
target is wqe->eth.inline_hdr.start (which the compiler sees as being
2 bytes in size), but copying 18, intending to write across start
(really vlan_tci, 2 bytes). The remaining 16 bytes get written into
wqe->data[0], covering byte_count (4 bytes), lkey (4 bytes), and addr
(8 bytes).
struct mlx5e_tx_wqe {
struct mlx5_wqe_ctrl_seg ctrl; /* 0 16 */
struct mlx5_wqe_eth_seg eth; /* 16 16 */
struct mlx5_wqe_data_seg data[]; /* 32 0 */
/* size: 32, cachelines: 1, members: 3 */
/* last cacheline: 32 bytes */
};
struct mlx5_wqe_eth_seg {
u8 swp_outer_l4_offset; /* 0 1 */
u8 swp_outer_l3_offset; /* 1 1 */
u8 swp_inner_l4_offset; /* 2 1 */
u8 swp_inner_l3_offset; /* 3 1 */
u8 cs_flags; /* 4 1 */
u8 swp_flags; /* 5 1 */
__be16 mss; /* 6 2 */
__be32 flow_table_metadata; /* 8 4 */
union {
struct {
__be16 sz; /* 12 2 */
u8 start[2]; /* 14 2 */
} inline_hdr; /* 12 4 */
struct {
__be16 type; /* 12 2 */
__be16 vlan_tci; /* 14 2 */
} insert; /* 12 4 */
__be32 trailer; /* 12 4 */
}; /* 12 4 */
/* size: 16, cachelines: 1, members: 9 */
/* last cacheline: 16 bytes */
};
struct mlx5_wqe_data_seg {
__be32 byte_count; /* 0 4 */
__be32 lkey; /* 4 4 */
__be64 addr; /* 8 8 */
/* size: 16, cachelines: 1, members: 3 */
/* last cacheline: 16 bytes */
};
So, split the memcpy() so the compiler can reason about the buffer
sizes.
"pahole" shows no size nor member offset changes to struct mlx5e_tx_wqe
nor struct mlx5e_umr_wqe. "objdump -d" shows no meaningful object
code changes (i.e. only source line number induced differences and
optimizations).
Cc: Saeed Mahameed <saeedm@nvidia.com>
Cc: Leon Romanovsky <leon@kernel.org>
Cc: "David S. Miller" <davem@davemloft.net>
Cc: Jakub Kicinski <kuba@kernel.org>
Cc: Alexei Starovoitov <ast@kernel.org>
Cc: Daniel Borkmann <daniel@iogearbox.net>
Cc: Jesper Dangaard Brouer <hawk@kernel.org>
Cc: John Fastabend <john.fastabend@gmail.com>
Cc: netdev@vger.kernel.org
Cc: linux-rdma@vger.kernel.org
Cc: bpf@vger.kernel.org
Signed-off-by: Kees Cook <keescook@chromium.org>
---
drivers/net/ethernet/mellanox/mlx5/core/en.h | 4 ++--
drivers/net/ethernet/mellanox/mlx5/core/en/xdp.c | 4 +++-
2 files changed, 5 insertions(+), 3 deletions(-)
diff --git a/drivers/net/ethernet/mellanox/mlx5/core/en.h b/drivers/net/ethernet/mellanox/mlx5/core/en.h
index 4f6897c1ea8d..8997476c20cc 100644
--- a/drivers/net/ethernet/mellanox/mlx5/core/en.h
+++ b/drivers/net/ethernet/mellanox/mlx5/core/en.h
@@ -200,7 +200,7 @@ static inline int mlx5e_get_max_num_channels(struct mlx5_core_dev *mdev)
struct mlx5e_tx_wqe {
struct mlx5_wqe_ctrl_seg ctrl;
struct mlx5_wqe_eth_seg eth;
- struct mlx5_wqe_data_seg data[0];
+ struct mlx5_wqe_data_seg data[];
};
struct mlx5e_rx_wqe_ll {
@@ -216,7 +216,7 @@ struct mlx5e_umr_wqe {
struct mlx5_wqe_ctrl_seg ctrl;
struct mlx5_wqe_umr_ctrl_seg uctrl;
struct mlx5_mkey_seg mkc;
- struct mlx5_mtt inline_mtts[0];
+ struct mlx5_mtt inline_mtts[];
};
extern const char mlx5e_self_tests[][ETH_GSTRING_LEN];
diff --git a/drivers/net/ethernet/mellanox/mlx5/core/en/xdp.c b/drivers/net/ethernet/mellanox/mlx5/core/en/xdp.c
index 2f0df5cc1a2d..efae2444c26f 100644
--- a/drivers/net/ethernet/mellanox/mlx5/core/en/xdp.c
+++ b/drivers/net/ethernet/mellanox/mlx5/core/en/xdp.c
@@ -341,8 +341,10 @@ mlx5e_xmit_xdp_frame(struct mlx5e_xdpsq *sq, struct mlx5e_xmit_data *xdptxd,
/* copy the inline part if required */
if (sq->min_inline_mode != MLX5_INLINE_MODE_NONE) {
- memcpy(eseg->inline_hdr.start, xdptxd->data, MLX5E_XDP_MIN_INLINE);
+ memcpy(eseg->inline_hdr.start, xdptxd->data, sizeof(eseg->inline_hdr.start));
eseg->inline_hdr.sz = cpu_to_be16(MLX5E_XDP_MIN_INLINE);
+ memcpy(dseg, xdptxd->data + sizeof(eseg->inline_hdr.start),
+ MLX5E_XDP_MIN_INLINE - sizeof(eseg->inline_hdr.start));
dma_len -= MLX5E_XDP_MIN_INLINE;
dma_addr += MLX5E_XDP_MIN_INLINE;
dseg++;
--
2.30.2
^ permalink raw reply related [flat|nested] 9+ messages in thread
* [PATCH 3/3] pcmcia: ray_cs: Split memcpy() to avoid bounds check warning
2021-08-19 20:28 [PATCH 0/3] net: Cleanups for FORTIFY_SOURCE Kees Cook
2021-08-19 20:28 ` [PATCH 1/3] ipw2x00: Avoid field-overflowing memcpy() Kees Cook
2021-08-19 20:28 ` [PATCH 2/3] net/mlx5e: " Kees Cook
@ 2021-08-19 20:28 ` Kees Cook
2021-08-20 17:01 ` [PATCH 0/3] net: Cleanups for FORTIFY_SOURCE Jakub Kicinski
3 siblings, 0 replies; 9+ messages in thread
From: Kees Cook @ 2021-08-19 20:28 UTC (permalink / raw)
To: netdev
Cc: Kees Cook, Kalle Valo, David S. Miller, Jakub Kicinski,
linux-wireless, Stanislav Yakovlev, Saeed Mahameed,
Leon Romanovsky, Alexei Starovoitov, Daniel Borkmann,
Jesper Dangaard Brouer, John Fastabend, Andrii Nakryiko,
Martin KaFai Lau, Song Liu, Yonghong Song, KP Singh,
linux-kernel, linux-rdma, bpf, linux-hardening
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 <kvalo@codeaurora.org>
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 <keescook@chromium.org>
---
drivers/net/wireless/ray_cs.c | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/drivers/net/wireless/ray_cs.c b/drivers/net/wireless/ray_cs.c
index 590bd974d94f..d57bbe551630 100644
--- a/drivers/net/wireless/ray_cs.c
+++ b/drivers/net/wireless/ray_cs.c
@@ -982,7 +982,9 @@ AP to AP 1 1 dest AP src AP dest source
if (local->net_type == ADHOC) {
writeb(0, &ptx->mac.frame_ctl_2);
memcpy_toio(ptx->mac.addr_1, ((struct ethhdr *)data)->h_dest,
- 2 * ADDRLEN);
+ ADDRLEN);
+ memcpy_toio(ptx->mac.addr_2, ((struct ethhdr *)data)->h_source,
+ ADDRLEN);
memcpy_toio(ptx->mac.addr_3, local->bss_id, ADDRLEN);
} else { /* infrastructure */
--
2.30.2
^ permalink raw reply related [flat|nested] 9+ messages in thread
* Re: [PATCH 0/3] net: Cleanups for FORTIFY_SOURCE
2021-08-19 20:28 [PATCH 0/3] net: Cleanups for FORTIFY_SOURCE Kees Cook
` (2 preceding siblings ...)
2021-08-19 20:28 ` [PATCH 3/3] pcmcia: ray_cs: Split memcpy() to avoid bounds check warning Kees Cook
@ 2021-08-20 17:01 ` Jakub Kicinski
2021-08-21 10:11 ` Kalle Valo
3 siblings, 1 reply; 9+ messages in thread
From: Jakub Kicinski @ 2021-08-20 17:01 UTC (permalink / raw)
To: Kees Cook, Saeed Mahameed, Kalle Valo
Cc: netdev, Stanislav Yakovlev, David S. Miller, Leon Romanovsky,
Alexei Starovoitov, Daniel Borkmann, Jesper Dangaard Brouer,
John Fastabend, Andrii Nakryiko, Martin KaFai Lau, Song Liu,
Yonghong Song, KP Singh, linux-kernel, linux-wireless,
linux-rdma, bpf, linux-hardening
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.
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH 0/3] net: Cleanups for FORTIFY_SOURCE
2021-08-20 17:01 ` [PATCH 0/3] net: Cleanups for FORTIFY_SOURCE Jakub Kicinski
@ 2021-08-21 10:11 ` Kalle Valo
2021-08-21 10:13 ` Kalle Valo
0 siblings, 1 reply; 9+ messages in thread
From: Kalle Valo @ 2021-08-21 10:11 UTC (permalink / raw)
To: Jakub Kicinski
Cc: Kees Cook, Saeed Mahameed, netdev, Stanislav Yakovlev,
David S. Miller, Leon Romanovsky, Alexei Starovoitov,
Daniel Borkmann, Jesper Dangaard Brouer, John Fastabend,
Andrii Nakryiko, Martin KaFai Lau, Song Liu, Yonghong Song,
KP Singh, linux-kernel, linux-wireless, linux-rdma, bpf,
linux-hardening
Jakub Kicinski <kuba@kernel.org> writes:
> 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.
Ok, I'll take patch 1 to wireless-drivers-next.
--
https://patchwork.kernel.org/project/linux-wireless/list/
https://wireless.wiki.kernel.org/en/developers/documentation/submittingpatches
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH 0/3] net: Cleanups for FORTIFY_SOURCE
2021-08-21 10:11 ` Kalle Valo
@ 2021-08-21 10:13 ` Kalle Valo
2021-08-22 5:16 ` Kees Cook
0 siblings, 1 reply; 9+ messages in thread
From: Kalle Valo @ 2021-08-21 10:13 UTC (permalink / raw)
To: Kalle Valo
Cc: Jakub Kicinski, Kees Cook, Saeed Mahameed, netdev,
Stanislav Yakovlev, David S. Miller, Leon Romanovsky,
Alexei Starovoitov, Daniel Borkmann, Jesper Dangaard Brouer,
John Fastabend, Andrii Nakryiko, Martin KaFai Lau, Song Liu,
Yonghong Song, KP Singh, linux-kernel, linux-wireless,
linux-rdma, bpf, linux-hardening
Kalle Valo <kvalo@codeaurora.org> writes:
> Jakub Kicinski <kuba@kernel.org> writes:
>
>> 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.
>
> Ok, I'll take patch 1 to wireless-drivers-next.
Correction: I'll take patches 1 and 3 to wireless-drivers-next.
--
https://patchwork.kernel.org/project/linux-wireless/list/
https://wireless.wiki.kernel.org/en/developers/documentation/submittingpatches
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH 1/3] ipw2x00: Avoid field-overflowing memcpy()
2021-08-19 20:28 ` [PATCH 1/3] ipw2x00: Avoid field-overflowing memcpy() Kees Cook
@ 2021-08-21 17:15 ` Kalle Valo
0 siblings, 0 replies; 9+ messages in thread
From: Kalle Valo @ 2021-08-21 17:15 UTC (permalink / raw)
To: Kees Cook
Cc: netdev, Kees Cook, Stanislav Yakovlev, David S. Miller,
Jakub Kicinski, linux-wireless, Saeed Mahameed, Leon Romanovsky,
Alexei Starovoitov, Daniel Borkmann, Jesper Dangaard Brouer,
John Fastabend, Andrii Nakryiko, Martin KaFai Lau, Song Liu,
Yonghong Song, KP Singh, linux-kernel, linux-rdma, bpf,
linux-hardening
Kees Cook <keescook@chromium.org> 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 <kvalo@codeaurora.org>
> 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 <keescook@chromium.org>
2 patches applied to wireless-drivers-next.git, thanks.
d6b6d1bb80be ipw2x00: Avoid field-overflowing memcpy()
92276c592a6b ray_cs: Split memcpy() to avoid bounds check warning
--
https://patchwork.kernel.org/project/linux-wireless/patch/20210819202825.3545692-2-keescook@chromium.org/
https://wireless.wiki.kernel.org/en/developers/documentation/submittingpatches
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH 0/3] net: Cleanups for FORTIFY_SOURCE
2021-08-21 10:13 ` Kalle Valo
@ 2021-08-22 5:16 ` Kees Cook
0 siblings, 0 replies; 9+ messages in thread
From: Kees Cook @ 2021-08-22 5:16 UTC (permalink / raw)
To: Kalle Valo
Cc: Jakub Kicinski, Saeed Mahameed, netdev, Stanislav Yakovlev,
David S. Miller, Leon Romanovsky, Alexei Starovoitov,
Daniel Borkmann, Jesper Dangaard Brouer, John Fastabend,
Andrii Nakryiko, Martin KaFai Lau, Song Liu, Yonghong Song,
KP Singh, linux-kernel, linux-wireless, linux-rdma, bpf,
linux-hardening
On Sat, Aug 21, 2021 at 01:13:37PM +0300, Kalle Valo wrote:
> Kalle Valo <kvalo@codeaurora.org> writes:
>
> > Jakub Kicinski <kuba@kernel.org> writes:
> >
> >> 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.
> >
> > Ok, I'll take patch 1 to wireless-drivers-next.
>
> Correction: I'll take patches 1 and 3 to wireless-drivers-next.
Great; thanks!
--
Kees Cook
^ permalink raw reply [flat|nested] 9+ messages in thread
end of thread, other threads:[~2021-08-22 5:16 UTC | newest]
Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-08-19 20:28 [PATCH 0/3] net: Cleanups for FORTIFY_SOURCE Kees Cook
2021-08-19 20:28 ` [PATCH 1/3] ipw2x00: Avoid field-overflowing memcpy() Kees Cook
2021-08-21 17:15 ` Kalle Valo
2021-08-19 20:28 ` [PATCH 2/3] net/mlx5e: " Kees Cook
2021-08-19 20:28 ` [PATCH 3/3] pcmcia: ray_cs: Split memcpy() to avoid bounds check warning Kees Cook
2021-08-20 17:01 ` [PATCH 0/3] net: Cleanups for FORTIFY_SOURCE Jakub Kicinski
2021-08-21 10:11 ` Kalle Valo
2021-08-21 10:13 ` Kalle Valo
2021-08-22 5:16 ` Kees Cook
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.