LKML Archive on lore.kernel.org
help / color / mirror / Atom feed
* [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	[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	[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	[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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).