All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH net-next v2 0/3] compiler_types: add Endianness-dependent __counted_by_{le,be}
@ 2024-03-27 14:22 ` Alexander Lobakin
  0 siblings, 0 replies; 12+ messages in thread
From: Alexander Lobakin @ 2024-03-27 14:22 UTC (permalink / raw)
  To: David S. Miller, Eric Dumazet, Jakub Kicinski, Paolo Abeni
  Cc: Alexander Lobakin, Kees Cook, Gustavo A. R. Silva,
	Nathan Chancellor, Simon Horman,
	nex.sw.ncis.osdt.itp.upstreaming, intel-wired-lan,
	linux-hardening, netdev, linux-kernel

Some structures contain flexible arrays at the end and the counter for
them, but the counter has explicit Endianness and thus __counted_by()
can't be used directly.

To increase test coverage for potential problems without breaking
anything, introduce __counted_by_{le,be} defined depending on platform's
Endianness to either __counted_by() when applicable or noop otherwise.
The first user will be virtchnl2.h from idpf just as example with 9 flex
structures having Little Endian counters.

Maybe it would be a good idea to introduce such attributes on compiler
level if possible, but for now let's stop on what we have.

Alexander Lobakin (3):
  compiler_types: add Endianness-dependent __counted_by_{le,be}
  idpf: make virtchnl2.h self-contained
  idpf: sprinkle __counted_by{,_le}() in the virtchnl2 header

 Documentation/conf.py                       |  2 ++
 scripts/kernel-doc                          |  1 +
 include/linux/compiler_types.h              | 11 ++++++++++
 drivers/net/ethernet/intel/idpf/idpf_txrx.h |  2 ++
 drivers/net/ethernet/intel/idpf/virtchnl2.h | 24 ++++++++++-----------
 5 files changed, 28 insertions(+), 12 deletions(-)

---
From v1[0]:
* fix compilation of #2 (Jakub);
* pick Acked-bys (Gustavo).

From RFC[1]:

* teach kdoc new attributes (Simon, Kees);
* add Acked-by (Kees);
* fix a couple typos;
* send to net-next (Kees).

[0] https://lore.kernel.org/netdev/20240326164116.645718-1-aleksander.lobakin@intel.com
[1] https://lore.kernel.org/netdev/20240318130354.2713265-1-aleksander.lobakin@intel.com
-- 
2.44.0


^ permalink raw reply	[flat|nested] 12+ messages in thread

* [Intel-wired-lan] [PATCH net-next v2 0/3] compiler_types: add Endianness-dependent __counted_by_{le, be}
@ 2024-03-27 14:22 ` Alexander Lobakin
  0 siblings, 0 replies; 12+ messages in thread
From: Alexander Lobakin @ 2024-03-27 14:22 UTC (permalink / raw)
  To: David S. Miller, Eric Dumazet, Jakub Kicinski, Paolo Abeni
  Cc: Kees Cook, netdev, Gustavo A. R. Silva, linux-kernel,
	Nathan Chancellor, Alexander Lobakin, intel-wired-lan,
	linux-hardening, Simon Horman, nex.sw.ncis.osdt.itp.upstreaming

Some structures contain flexible arrays at the end and the counter for
them, but the counter has explicit Endianness and thus __counted_by()
can't be used directly.

To increase test coverage for potential problems without breaking
anything, introduce __counted_by_{le,be} defined depending on platform's
Endianness to either __counted_by() when applicable or noop otherwise.
The first user will be virtchnl2.h from idpf just as example with 9 flex
structures having Little Endian counters.

Maybe it would be a good idea to introduce such attributes on compiler
level if possible, but for now let's stop on what we have.

Alexander Lobakin (3):
  compiler_types: add Endianness-dependent __counted_by_{le,be}
  idpf: make virtchnl2.h self-contained
  idpf: sprinkle __counted_by{,_le}() in the virtchnl2 header

 Documentation/conf.py                       |  2 ++
 scripts/kernel-doc                          |  1 +
 include/linux/compiler_types.h              | 11 ++++++++++
 drivers/net/ethernet/intel/idpf/idpf_txrx.h |  2 ++
 drivers/net/ethernet/intel/idpf/virtchnl2.h | 24 ++++++++++-----------
 5 files changed, 28 insertions(+), 12 deletions(-)

---
From v1[0]:
* fix compilation of #2 (Jakub);
* pick Acked-bys (Gustavo).

From RFC[1]:

* teach kdoc new attributes (Simon, Kees);
* add Acked-by (Kees);
* fix a couple typos;
* send to net-next (Kees).

[0] https://lore.kernel.org/netdev/20240326164116.645718-1-aleksander.lobakin@intel.com
[1] https://lore.kernel.org/netdev/20240318130354.2713265-1-aleksander.lobakin@intel.com
-- 
2.44.0


^ permalink raw reply	[flat|nested] 12+ messages in thread

* [PATCH net-next v2 1/3] compiler_types: add Endianness-dependent __counted_by_{le,be}
  2024-03-27 14:22 ` [Intel-wired-lan] [PATCH net-next v2 0/3] compiler_types: add Endianness-dependent __counted_by_{le, be} Alexander Lobakin
@ 2024-03-27 14:22   ` Alexander Lobakin
  -1 siblings, 0 replies; 12+ messages in thread
From: Alexander Lobakin @ 2024-03-27 14:22 UTC (permalink / raw)
  To: David S. Miller, Eric Dumazet, Jakub Kicinski, Paolo Abeni
  Cc: Alexander Lobakin, Kees Cook, Gustavo A. R. Silva,
	Nathan Chancellor, Simon Horman,
	nex.sw.ncis.osdt.itp.upstreaming, intel-wired-lan,
	linux-hardening, netdev, linux-kernel

Some structures contain flexible arrays at the end and the counter for
them, but the counter has explicit Endianness and thus __counted_by()
can't be used directly.

To increase test coverage for potential problems without breaking
anything, introduce __counted_by_{le,be}() defined depending on
platform's Endianness to either __counted_by() when applicable or noop
otherwise.
Maybe it would be a good idea to introduce such attributes on compiler
level if possible, but for now let's stop on what we have.

Acked-by: Kees Cook <keescook@chromium.org>
Acked-by: Gustavo A. R. Silva <gustavoars@kernel.org>
Signed-off-by: Alexander Lobakin <aleksander.lobakin@intel.com>
---
 Documentation/conf.py          |  2 ++
 scripts/kernel-doc             |  1 +
 include/linux/compiler_types.h | 11 +++++++++++
 3 files changed, 14 insertions(+)

diff --git a/Documentation/conf.py b/Documentation/conf.py
index d148f3e8dd57..0c2205d536b3 100644
--- a/Documentation/conf.py
+++ b/Documentation/conf.py
@@ -75,6 +75,8 @@ if major >= 3:
             "__rcu",
             "__user",
             "__force",
+            "__counted_by_le",
+            "__counted_by_be",
 
             # include/linux/compiler_attributes.h:
             "__alias",
diff --git a/scripts/kernel-doc b/scripts/kernel-doc
index 967f1abb0edb..1474e95dbe4f 100755
--- a/scripts/kernel-doc
+++ b/scripts/kernel-doc
@@ -1143,6 +1143,7 @@ sub dump_struct($$) {
         $members =~ s/\s*$attribute/ /gi;
         $members =~ s/\s*__aligned\s*\([^;]*\)/ /gos;
         $members =~ s/\s*__counted_by\s*\([^;]*\)/ /gos;
+        $members =~ s/\s*__counted_by_(le|be)\s*\([^;]*\)/ /gos;
         $members =~ s/\s*__packed\s*/ /gos;
         $members =~ s/\s*CRYPTO_MINALIGN_ATTR/ /gos;
         $members =~ s/\s*____cacheline_aligned_in_smp/ /gos;
diff --git a/include/linux/compiler_types.h b/include/linux/compiler_types.h
index 2abaa3a825a9..a29ba6ef1e27 100644
--- a/include/linux/compiler_types.h
+++ b/include/linux/compiler_types.h
@@ -282,6 +282,17 @@ struct ftrace_likely_data {
 #define __no_sanitize_or_inline __always_inline
 #endif
 
+/*
+ * Apply __counted_by() when the Endianness matches to increase test coverage.
+ */
+#ifdef __LITTLE_ENDIAN
+#define __counted_by_le(member)	__counted_by(member)
+#define __counted_by_be(member)
+#else
+#define __counted_by_le(member)
+#define __counted_by_be(member)	__counted_by(member)
+#endif
+
 /* Do not trap wrapping arithmetic within an annotated function. */
 #ifdef CONFIG_UBSAN_SIGNED_WRAP
 # define __signed_wrap __attribute__((no_sanitize("signed-integer-overflow")))
-- 
2.44.0


^ permalink raw reply related	[flat|nested] 12+ messages in thread

* [Intel-wired-lan] [PATCH net-next v2 1/3] compiler_types: add Endianness-dependent __counted_by_{le, be}
@ 2024-03-27 14:22   ` Alexander Lobakin
  0 siblings, 0 replies; 12+ messages in thread
From: Alexander Lobakin @ 2024-03-27 14:22 UTC (permalink / raw)
  To: David S. Miller, Eric Dumazet, Jakub Kicinski, Paolo Abeni
  Cc: Kees Cook, netdev, Gustavo A. R. Silva, linux-kernel,
	Nathan Chancellor, Alexander Lobakin, intel-wired-lan,
	linux-hardening, Simon Horman, nex.sw.ncis.osdt.itp.upstreaming

Some structures contain flexible arrays at the end and the counter for
them, but the counter has explicit Endianness and thus __counted_by()
can't be used directly.

To increase test coverage for potential problems without breaking
anything, introduce __counted_by_{le,be}() defined depending on
platform's Endianness to either __counted_by() when applicable or noop
otherwise.
Maybe it would be a good idea to introduce such attributes on compiler
level if possible, but for now let's stop on what we have.

Acked-by: Kees Cook <keescook@chromium.org>
Acked-by: Gustavo A. R. Silva <gustavoars@kernel.org>
Signed-off-by: Alexander Lobakin <aleksander.lobakin@intel.com>
---
 Documentation/conf.py          |  2 ++
 scripts/kernel-doc             |  1 +
 include/linux/compiler_types.h | 11 +++++++++++
 3 files changed, 14 insertions(+)

diff --git a/Documentation/conf.py b/Documentation/conf.py
index d148f3e8dd57..0c2205d536b3 100644
--- a/Documentation/conf.py
+++ b/Documentation/conf.py
@@ -75,6 +75,8 @@ if major >= 3:
             "__rcu",
             "__user",
             "__force",
+            "__counted_by_le",
+            "__counted_by_be",
 
             # include/linux/compiler_attributes.h:
             "__alias",
diff --git a/scripts/kernel-doc b/scripts/kernel-doc
index 967f1abb0edb..1474e95dbe4f 100755
--- a/scripts/kernel-doc
+++ b/scripts/kernel-doc
@@ -1143,6 +1143,7 @@ sub dump_struct($$) {
         $members =~ s/\s*$attribute/ /gi;
         $members =~ s/\s*__aligned\s*\([^;]*\)/ /gos;
         $members =~ s/\s*__counted_by\s*\([^;]*\)/ /gos;
+        $members =~ s/\s*__counted_by_(le|be)\s*\([^;]*\)/ /gos;
         $members =~ s/\s*__packed\s*/ /gos;
         $members =~ s/\s*CRYPTO_MINALIGN_ATTR/ /gos;
         $members =~ s/\s*____cacheline_aligned_in_smp/ /gos;
diff --git a/include/linux/compiler_types.h b/include/linux/compiler_types.h
index 2abaa3a825a9..a29ba6ef1e27 100644
--- a/include/linux/compiler_types.h
+++ b/include/linux/compiler_types.h
@@ -282,6 +282,17 @@ struct ftrace_likely_data {
 #define __no_sanitize_or_inline __always_inline
 #endif
 
+/*
+ * Apply __counted_by() when the Endianness matches to increase test coverage.
+ */
+#ifdef __LITTLE_ENDIAN
+#define __counted_by_le(member)	__counted_by(member)
+#define __counted_by_be(member)
+#else
+#define __counted_by_le(member)
+#define __counted_by_be(member)	__counted_by(member)
+#endif
+
 /* Do not trap wrapping arithmetic within an annotated function. */
 #ifdef CONFIG_UBSAN_SIGNED_WRAP
 # define __signed_wrap __attribute__((no_sanitize("signed-integer-overflow")))
-- 
2.44.0


^ permalink raw reply related	[flat|nested] 12+ messages in thread

* [PATCH net-next v2 2/3] idpf: make virtchnl2.h self-contained
  2024-03-27 14:22 ` [Intel-wired-lan] [PATCH net-next v2 0/3] compiler_types: add Endianness-dependent __counted_by_{le, be} Alexander Lobakin
@ 2024-03-27 14:22   ` Alexander Lobakin
  -1 siblings, 0 replies; 12+ messages in thread
From: Alexander Lobakin @ 2024-03-27 14:22 UTC (permalink / raw)
  To: David S. Miller, Eric Dumazet, Jakub Kicinski, Paolo Abeni
  Cc: Alexander Lobakin, Kees Cook, Gustavo A. R. Silva,
	Nathan Chancellor, Simon Horman,
	nex.sw.ncis.osdt.itp.upstreaming, intel-wired-lan,
	linux-hardening, netdev, linux-kernel

To ease maintaining of virtchnl2.h, which already is messy enough,
make it self-contained by adding missing if_ether.h include due to
%ETH_ALEN usage.
At the same time, virtchnl2_lan_desc.h is not used anywhere in the
file, so move this include to idpf_txrx.h to speed up C preprocessing.

Acked-by: Kees Cook <keescook@chromium.org>
Acked-by: Gustavo A. R. Silva <gustavoars@kernel.org>
Signed-off-by: Alexander Lobakin <aleksander.lobakin@intel.com>
---
 drivers/net/ethernet/intel/idpf/idpf_txrx.h | 2 ++
 drivers/net/ethernet/intel/idpf/virtchnl2.h | 4 ++--
 2 files changed, 4 insertions(+), 2 deletions(-)

diff --git a/drivers/net/ethernet/intel/idpf/idpf_txrx.h b/drivers/net/ethernet/intel/idpf/idpf_txrx.h
index df76493faa75..3d046b81e507 100644
--- a/drivers/net/ethernet/intel/idpf/idpf_txrx.h
+++ b/drivers/net/ethernet/intel/idpf/idpf_txrx.h
@@ -8,6 +8,8 @@
 #include <net/tcp.h>
 #include <net/netdev_queues.h>
 
+#include "virtchnl2_lan_desc.h"
+
 #define IDPF_LARGE_MAX_Q			256
 #define IDPF_MAX_Q				16
 #define IDPF_MIN_Q				2
diff --git a/drivers/net/ethernet/intel/idpf/virtchnl2.h b/drivers/net/ethernet/intel/idpf/virtchnl2.h
index 4a3c4454d25a..29419211b3d9 100644
--- a/drivers/net/ethernet/intel/idpf/virtchnl2.h
+++ b/drivers/net/ethernet/intel/idpf/virtchnl2.h
@@ -4,6 +4,8 @@
 #ifndef _VIRTCHNL2_H_
 #define _VIRTCHNL2_H_
 
+#include <linux/if_ether.h>
+
 /* All opcodes associated with virtchnl2 are prefixed with virtchnl2 or
  * VIRTCHNL2. Any future opcodes, offloads/capabilities, structures,
  * and defines must be prefixed with virtchnl2 or VIRTCHNL2 to avoid confusion.
@@ -17,8 +19,6 @@
  * must remain unchanged over time, so we specify explicit values for all enums.
  */
 
-#include "virtchnl2_lan_desc.h"
-
 /* This macro is used to generate compilation errors if a structure
  * is not exactly the correct length.
  */
-- 
2.44.0


^ permalink raw reply related	[flat|nested] 12+ messages in thread

* [Intel-wired-lan] [PATCH net-next v2 2/3] idpf: make virtchnl2.h self-contained
@ 2024-03-27 14:22   ` Alexander Lobakin
  0 siblings, 0 replies; 12+ messages in thread
From: Alexander Lobakin @ 2024-03-27 14:22 UTC (permalink / raw)
  To: David S. Miller, Eric Dumazet, Jakub Kicinski, Paolo Abeni
  Cc: Kees Cook, netdev, Gustavo A. R. Silva, linux-kernel,
	Nathan Chancellor, Alexander Lobakin, intel-wired-lan,
	linux-hardening, Simon Horman, nex.sw.ncis.osdt.itp.upstreaming

To ease maintaining of virtchnl2.h, which already is messy enough,
make it self-contained by adding missing if_ether.h include due to
%ETH_ALEN usage.
At the same time, virtchnl2_lan_desc.h is not used anywhere in the
file, so move this include to idpf_txrx.h to speed up C preprocessing.

Acked-by: Kees Cook <keescook@chromium.org>
Acked-by: Gustavo A. R. Silva <gustavoars@kernel.org>
Signed-off-by: Alexander Lobakin <aleksander.lobakin@intel.com>
---
 drivers/net/ethernet/intel/idpf/idpf_txrx.h | 2 ++
 drivers/net/ethernet/intel/idpf/virtchnl2.h | 4 ++--
 2 files changed, 4 insertions(+), 2 deletions(-)

diff --git a/drivers/net/ethernet/intel/idpf/idpf_txrx.h b/drivers/net/ethernet/intel/idpf/idpf_txrx.h
index df76493faa75..3d046b81e507 100644
--- a/drivers/net/ethernet/intel/idpf/idpf_txrx.h
+++ b/drivers/net/ethernet/intel/idpf/idpf_txrx.h
@@ -8,6 +8,8 @@
 #include <net/tcp.h>
 #include <net/netdev_queues.h>
 
+#include "virtchnl2_lan_desc.h"
+
 #define IDPF_LARGE_MAX_Q			256
 #define IDPF_MAX_Q				16
 #define IDPF_MIN_Q				2
diff --git a/drivers/net/ethernet/intel/idpf/virtchnl2.h b/drivers/net/ethernet/intel/idpf/virtchnl2.h
index 4a3c4454d25a..29419211b3d9 100644
--- a/drivers/net/ethernet/intel/idpf/virtchnl2.h
+++ b/drivers/net/ethernet/intel/idpf/virtchnl2.h
@@ -4,6 +4,8 @@
 #ifndef _VIRTCHNL2_H_
 #define _VIRTCHNL2_H_
 
+#include <linux/if_ether.h>
+
 /* All opcodes associated with virtchnl2 are prefixed with virtchnl2 or
  * VIRTCHNL2. Any future opcodes, offloads/capabilities, structures,
  * and defines must be prefixed with virtchnl2 or VIRTCHNL2 to avoid confusion.
@@ -17,8 +19,6 @@
  * must remain unchanged over time, so we specify explicit values for all enums.
  */
 
-#include "virtchnl2_lan_desc.h"
-
 /* This macro is used to generate compilation errors if a structure
  * is not exactly the correct length.
  */
-- 
2.44.0


^ permalink raw reply related	[flat|nested] 12+ messages in thread

* [PATCH net-next v2 3/3] idpf: sprinkle __counted_by{,_le}() in the virtchnl2 header
  2024-03-27 14:22 ` [Intel-wired-lan] [PATCH net-next v2 0/3] compiler_types: add Endianness-dependent __counted_by_{le, be} Alexander Lobakin
@ 2024-03-27 14:22   ` Alexander Lobakin
  -1 siblings, 0 replies; 12+ messages in thread
From: Alexander Lobakin @ 2024-03-27 14:22 UTC (permalink / raw)
  To: David S. Miller, Eric Dumazet, Jakub Kicinski, Paolo Abeni
  Cc: Alexander Lobakin, Kees Cook, Gustavo A. R. Silva,
	Nathan Chancellor, Simon Horman,
	nex.sw.ncis.osdt.itp.upstreaming, intel-wired-lan,
	linux-hardening, netdev, linux-kernel

Both virtchnl2.h and its consumer idpf_virtchnl.c are very error-prone.
There are 10 structures with flexible arrays at the end, but 9 of them
has flex member counter in Little Endian.
Make the code a bit more robust by applying __counted_by_le() to those
9. LE platforms is the main target for this driver, so they would
receive additional protection.
While we're here, add __counted_by() to virtchnl2_ptype::proto_id, as
its counter is `u8` regardless of the Endianness.
Compile test on x86_64 (LE) didn't reveal any new issues after applying
the attributes.

Acked-by: Kees Cook <keescook@chromium.org>
Acked-by: Gustavo A. R. Silva <gustavoars@kernel.org>
Signed-off-by: Alexander Lobakin <aleksander.lobakin@intel.com>
---
 drivers/net/ethernet/intel/idpf/virtchnl2.h | 20 ++++++++++----------
 1 file changed, 10 insertions(+), 10 deletions(-)

diff --git a/drivers/net/ethernet/intel/idpf/virtchnl2.h b/drivers/net/ethernet/intel/idpf/virtchnl2.h
index 29419211b3d9..63deb120359c 100644
--- a/drivers/net/ethernet/intel/idpf/virtchnl2.h
+++ b/drivers/net/ethernet/intel/idpf/virtchnl2.h
@@ -555,7 +555,7 @@ VIRTCHNL2_CHECK_STRUCT_LEN(32, virtchnl2_queue_reg_chunk);
 struct virtchnl2_queue_reg_chunks {
 	__le16 num_chunks;
 	u8 pad[6];
-	struct virtchnl2_queue_reg_chunk chunks[];
+	struct virtchnl2_queue_reg_chunk chunks[] __counted_by_le(num_chunks);
 };
 VIRTCHNL2_CHECK_STRUCT_LEN(8, virtchnl2_queue_reg_chunks);
 
@@ -703,7 +703,7 @@ struct virtchnl2_config_tx_queues {
 	__le32 vport_id;
 	__le16 num_qinfo;
 	u8 pad[10];
-	struct virtchnl2_txq_info qinfo[];
+	struct virtchnl2_txq_info qinfo[] __counted_by_le(num_qinfo);
 };
 VIRTCHNL2_CHECK_STRUCT_LEN(16, virtchnl2_config_tx_queues);
 
@@ -782,7 +782,7 @@ struct virtchnl2_config_rx_queues {
 	__le32 vport_id;
 	__le16 num_qinfo;
 	u8 pad[18];
-	struct virtchnl2_rxq_info qinfo[];
+	struct virtchnl2_rxq_info qinfo[] __counted_by_le(num_qinfo);
 };
 VIRTCHNL2_CHECK_STRUCT_LEN(24, virtchnl2_config_rx_queues);
 
@@ -868,7 +868,7 @@ VIRTCHNL2_CHECK_STRUCT_LEN(32, virtchnl2_vector_chunk);
 struct virtchnl2_vector_chunks {
 	__le16 num_vchunks;
 	u8 pad[14];
-	struct virtchnl2_vector_chunk vchunks[];
+	struct virtchnl2_vector_chunk vchunks[] __counted_by_le(num_vchunks);
 };
 VIRTCHNL2_CHECK_STRUCT_LEN(16, virtchnl2_vector_chunks);
 
@@ -912,7 +912,7 @@ struct virtchnl2_rss_lut {
 	__le16 lut_entries_start;
 	__le16 lut_entries;
 	u8 pad[4];
-	__le32 lut[];
+	__le32 lut[] __counted_by_le(lut_entries);
 };
 VIRTCHNL2_CHECK_STRUCT_LEN(12, virtchnl2_rss_lut);
 
@@ -977,7 +977,7 @@ struct virtchnl2_ptype {
 	u8 ptype_id_8;
 	u8 proto_id_count;
 	__le16 pad;
-	__le16 proto_id[];
+	__le16 proto_id[] __counted_by(proto_id_count);
 } __packed __aligned(2);
 VIRTCHNL2_CHECK_STRUCT_LEN(6, virtchnl2_ptype);
 
@@ -1104,7 +1104,7 @@ struct virtchnl2_rss_key {
 	__le32 vport_id;
 	__le16 key_len;
 	u8 pad;
-	u8 key_flex[];
+	u8 key_flex[] __counted_by_le(key_len);
 } __packed;
 VIRTCHNL2_CHECK_STRUCT_LEN(7, virtchnl2_rss_key);
 
@@ -1131,7 +1131,7 @@ VIRTCHNL2_CHECK_STRUCT_LEN(16, virtchnl2_queue_chunk);
 struct virtchnl2_queue_chunks {
 	__le16 num_chunks;
 	u8 pad[6];
-	struct virtchnl2_queue_chunk chunks[];
+	struct virtchnl2_queue_chunk chunks[] __counted_by_le(num_chunks);
 };
 VIRTCHNL2_CHECK_STRUCT_LEN(8, virtchnl2_queue_chunks);
 
@@ -1195,7 +1195,7 @@ struct virtchnl2_queue_vector_maps {
 	__le32 vport_id;
 	__le16 num_qv_maps;
 	u8 pad[10];
-	struct virtchnl2_queue_vector qv_maps[];
+	struct virtchnl2_queue_vector qv_maps[] __counted_by_le(num_qv_maps);
 };
 VIRTCHNL2_CHECK_STRUCT_LEN(16, virtchnl2_queue_vector_maps);
 
@@ -1247,7 +1247,7 @@ struct virtchnl2_mac_addr_list {
 	__le32 vport_id;
 	__le16 num_mac_addr;
 	u8 pad[2];
-	struct virtchnl2_mac_addr mac_addr_list[];
+	struct virtchnl2_mac_addr mac_addr_list[] __counted_by_le(num_mac_addr);
 };
 VIRTCHNL2_CHECK_STRUCT_LEN(8, virtchnl2_mac_addr_list);
 
-- 
2.44.0


^ permalink raw reply related	[flat|nested] 12+ messages in thread

* [Intel-wired-lan] [PATCH net-next v2 3/3] idpf: sprinkle __counted_by{, _le}() in the virtchnl2 header
@ 2024-03-27 14:22   ` Alexander Lobakin
  0 siblings, 0 replies; 12+ messages in thread
From: Alexander Lobakin @ 2024-03-27 14:22 UTC (permalink / raw)
  To: David S. Miller, Eric Dumazet, Jakub Kicinski, Paolo Abeni
  Cc: Kees Cook, netdev, Gustavo A. R. Silva, linux-kernel,
	Nathan Chancellor, Alexander Lobakin, intel-wired-lan,
	linux-hardening, Simon Horman, nex.sw.ncis.osdt.itp.upstreaming

Both virtchnl2.h and its consumer idpf_virtchnl.c are very error-prone.
There are 10 structures with flexible arrays at the end, but 9 of them
has flex member counter in Little Endian.
Make the code a bit more robust by applying __counted_by_le() to those
9. LE platforms is the main target for this driver, so they would
receive additional protection.
While we're here, add __counted_by() to virtchnl2_ptype::proto_id, as
its counter is `u8` regardless of the Endianness.
Compile test on x86_64 (LE) didn't reveal any new issues after applying
the attributes.

Acked-by: Kees Cook <keescook@chromium.org>
Acked-by: Gustavo A. R. Silva <gustavoars@kernel.org>
Signed-off-by: Alexander Lobakin <aleksander.lobakin@intel.com>
---
 drivers/net/ethernet/intel/idpf/virtchnl2.h | 20 ++++++++++----------
 1 file changed, 10 insertions(+), 10 deletions(-)

diff --git a/drivers/net/ethernet/intel/idpf/virtchnl2.h b/drivers/net/ethernet/intel/idpf/virtchnl2.h
index 29419211b3d9..63deb120359c 100644
--- a/drivers/net/ethernet/intel/idpf/virtchnl2.h
+++ b/drivers/net/ethernet/intel/idpf/virtchnl2.h
@@ -555,7 +555,7 @@ VIRTCHNL2_CHECK_STRUCT_LEN(32, virtchnl2_queue_reg_chunk);
 struct virtchnl2_queue_reg_chunks {
 	__le16 num_chunks;
 	u8 pad[6];
-	struct virtchnl2_queue_reg_chunk chunks[];
+	struct virtchnl2_queue_reg_chunk chunks[] __counted_by_le(num_chunks);
 };
 VIRTCHNL2_CHECK_STRUCT_LEN(8, virtchnl2_queue_reg_chunks);
 
@@ -703,7 +703,7 @@ struct virtchnl2_config_tx_queues {
 	__le32 vport_id;
 	__le16 num_qinfo;
 	u8 pad[10];
-	struct virtchnl2_txq_info qinfo[];
+	struct virtchnl2_txq_info qinfo[] __counted_by_le(num_qinfo);
 };
 VIRTCHNL2_CHECK_STRUCT_LEN(16, virtchnl2_config_tx_queues);
 
@@ -782,7 +782,7 @@ struct virtchnl2_config_rx_queues {
 	__le32 vport_id;
 	__le16 num_qinfo;
 	u8 pad[18];
-	struct virtchnl2_rxq_info qinfo[];
+	struct virtchnl2_rxq_info qinfo[] __counted_by_le(num_qinfo);
 };
 VIRTCHNL2_CHECK_STRUCT_LEN(24, virtchnl2_config_rx_queues);
 
@@ -868,7 +868,7 @@ VIRTCHNL2_CHECK_STRUCT_LEN(32, virtchnl2_vector_chunk);
 struct virtchnl2_vector_chunks {
 	__le16 num_vchunks;
 	u8 pad[14];
-	struct virtchnl2_vector_chunk vchunks[];
+	struct virtchnl2_vector_chunk vchunks[] __counted_by_le(num_vchunks);
 };
 VIRTCHNL2_CHECK_STRUCT_LEN(16, virtchnl2_vector_chunks);
 
@@ -912,7 +912,7 @@ struct virtchnl2_rss_lut {
 	__le16 lut_entries_start;
 	__le16 lut_entries;
 	u8 pad[4];
-	__le32 lut[];
+	__le32 lut[] __counted_by_le(lut_entries);
 };
 VIRTCHNL2_CHECK_STRUCT_LEN(12, virtchnl2_rss_lut);
 
@@ -977,7 +977,7 @@ struct virtchnl2_ptype {
 	u8 ptype_id_8;
 	u8 proto_id_count;
 	__le16 pad;
-	__le16 proto_id[];
+	__le16 proto_id[] __counted_by(proto_id_count);
 } __packed __aligned(2);
 VIRTCHNL2_CHECK_STRUCT_LEN(6, virtchnl2_ptype);
 
@@ -1104,7 +1104,7 @@ struct virtchnl2_rss_key {
 	__le32 vport_id;
 	__le16 key_len;
 	u8 pad;
-	u8 key_flex[];
+	u8 key_flex[] __counted_by_le(key_len);
 } __packed;
 VIRTCHNL2_CHECK_STRUCT_LEN(7, virtchnl2_rss_key);
 
@@ -1131,7 +1131,7 @@ VIRTCHNL2_CHECK_STRUCT_LEN(16, virtchnl2_queue_chunk);
 struct virtchnl2_queue_chunks {
 	__le16 num_chunks;
 	u8 pad[6];
-	struct virtchnl2_queue_chunk chunks[];
+	struct virtchnl2_queue_chunk chunks[] __counted_by_le(num_chunks);
 };
 VIRTCHNL2_CHECK_STRUCT_LEN(8, virtchnl2_queue_chunks);
 
@@ -1195,7 +1195,7 @@ struct virtchnl2_queue_vector_maps {
 	__le32 vport_id;
 	__le16 num_qv_maps;
 	u8 pad[10];
-	struct virtchnl2_queue_vector qv_maps[];
+	struct virtchnl2_queue_vector qv_maps[] __counted_by_le(num_qv_maps);
 };
 VIRTCHNL2_CHECK_STRUCT_LEN(16, virtchnl2_queue_vector_maps);
 
@@ -1247,7 +1247,7 @@ struct virtchnl2_mac_addr_list {
 	__le32 vport_id;
 	__le16 num_mac_addr;
 	u8 pad[2];
-	struct virtchnl2_mac_addr mac_addr_list[];
+	struct virtchnl2_mac_addr mac_addr_list[] __counted_by_le(num_mac_addr);
 };
 VIRTCHNL2_CHECK_STRUCT_LEN(8, virtchnl2_mac_addr_list);
 
-- 
2.44.0


^ permalink raw reply related	[flat|nested] 12+ messages in thread

* Re: [PATCH net-next v2 0/3] compiler_types: add Endianness-dependent __counted_by_{le,be}
  2024-03-27 14:22 ` [Intel-wired-lan] [PATCH net-next v2 0/3] compiler_types: add Endianness-dependent __counted_by_{le, be} Alexander Lobakin
@ 2024-03-28 10:11   ` Przemek Kitszel
  -1 siblings, 0 replies; 12+ messages in thread
From: Przemek Kitszel @ 2024-03-28 10:11 UTC (permalink / raw)
  To: Alexander Lobakin, David S. Miller, Eric Dumazet, Jakub Kicinski,
	Paolo Abeni
  Cc: Kees Cook, Gustavo A. R. Silva, Nathan Chancellor, Simon Horman,
	nex.sw.ncis.osdt.itp.upstreaming, intel-wired-lan,
	linux-hardening, netdev, linux-kernel

On 3/27/24 15:22, Alexander Lobakin wrote:
> Some structures contain flexible arrays at the end and the counter for
> them, but the counter has explicit Endianness and thus __counted_by()
> can't be used directly.
> 
> To increase test coverage for potential problems without breaking
> anything, introduce __counted_by_{le,be} defined depending on platform's
> Endianness to either __counted_by() when applicable or noop otherwise.
> The first user will be virtchnl2.h from idpf just as example with 9 flex
> structures having Little Endian counters.
> 
> Maybe it would be a good idea to introduce such attributes on compiler
> level if possible, but for now let's stop on what we have.
> 
> Alexander Lobakin (3):
>    compiler_types: add Endianness-dependent __counted_by_{le,be}
>    idpf: make virtchnl2.h self-contained
>    idpf: sprinkle __counted_by{,_le}() in the virtchnl2 header
> 
>   Documentation/conf.py                       |  2 ++
>   scripts/kernel-doc                          |  1 +
>   include/linux/compiler_types.h              | 11 ++++++++++
>   drivers/net/ethernet/intel/idpf/idpf_txrx.h |  2 ++
>   drivers/net/ethernet/intel/idpf/virtchnl2.h | 24 ++++++++++-----------
>   5 files changed, 28 insertions(+), 12 deletions(-)
> 
> ---
>  From v1[0]:
> * fix compilation of #2 (Jakub);
> * pick Acked-bys (Gustavo).
> 
>  From RFC[1]:
> 
> * teach kdoc new attributes (Simon, Kees);
> * add Acked-by (Kees);
> * fix a couple typos;
> * send to net-next (Kees).
> 
> [0] https://lore.kernel.org/netdev/20240326164116.645718-1-aleksander.lobakin@intel.com
> [1] https://lore.kernel.org/netdev/20240318130354.2713265-1-aleksander.lobakin@intel.com

nice,
Reviewed-by: Przemek Kitszel <przemyslaw.kitszel@intel.com>

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [Intel-wired-lan] [PATCH net-next v2 0/3] compiler_types: add Endianness-dependent __counted_by_{le, be}
@ 2024-03-28 10:11   ` Przemek Kitszel
  0 siblings, 0 replies; 12+ messages in thread
From: Przemek Kitszel @ 2024-03-28 10:11 UTC (permalink / raw)
  To: Alexander Lobakin, David S. Miller, Eric Dumazet, Jakub Kicinski,
	Paolo Abeni
  Cc: Kees Cook, netdev, Gustavo A. R. Silva, linux-kernel,
	Nathan Chancellor, intel-wired-lan, linux-hardening,
	Simon Horman, nex.sw.ncis.osdt.itp.upstreaming

On 3/27/24 15:22, Alexander Lobakin wrote:
> Some structures contain flexible arrays at the end and the counter for
> them, but the counter has explicit Endianness and thus __counted_by()
> can't be used directly.
> 
> To increase test coverage for potential problems without breaking
> anything, introduce __counted_by_{le,be} defined depending on platform's
> Endianness to either __counted_by() when applicable or noop otherwise.
> The first user will be virtchnl2.h from idpf just as example with 9 flex
> structures having Little Endian counters.
> 
> Maybe it would be a good idea to introduce such attributes on compiler
> level if possible, but for now let's stop on what we have.
> 
> Alexander Lobakin (3):
>    compiler_types: add Endianness-dependent __counted_by_{le,be}
>    idpf: make virtchnl2.h self-contained
>    idpf: sprinkle __counted_by{,_le}() in the virtchnl2 header
> 
>   Documentation/conf.py                       |  2 ++
>   scripts/kernel-doc                          |  1 +
>   include/linux/compiler_types.h              | 11 ++++++++++
>   drivers/net/ethernet/intel/idpf/idpf_txrx.h |  2 ++
>   drivers/net/ethernet/intel/idpf/virtchnl2.h | 24 ++++++++++-----------
>   5 files changed, 28 insertions(+), 12 deletions(-)
> 
> ---
>  From v1[0]:
> * fix compilation of #2 (Jakub);
> * pick Acked-bys (Gustavo).
> 
>  From RFC[1]:
> 
> * teach kdoc new attributes (Simon, Kees);
> * add Acked-by (Kees);
> * fix a couple typos;
> * send to net-next (Kees).
> 
> [0] https://lore.kernel.org/netdev/20240326164116.645718-1-aleksander.lobakin@intel.com
> [1] https://lore.kernel.org/netdev/20240318130354.2713265-1-aleksander.lobakin@intel.com

nice,
Reviewed-by: Przemek Kitszel <przemyslaw.kitszel@intel.com>

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [Intel-wired-lan] [PATCH net-next v2 0/3] compiler_types: add Endianness-dependent __counted_by_{le, be}
  2024-03-27 14:22 ` [Intel-wired-lan] [PATCH net-next v2 0/3] compiler_types: add Endianness-dependent __counted_by_{le, be} Alexander Lobakin
@ 2024-03-29  2:20   ` patchwork-bot+netdevbpf
  -1 siblings, 0 replies; 12+ messages in thread
From: patchwork-bot+netdevbpf @ 2024-03-29  2:20 UTC (permalink / raw)
  To: Alexander Lobakin
  Cc: keescook, netdev, gustavoars, linux-kernel, nathan, edumazet,
	intel-wired-lan, linux-hardening, horms,
	nex.sw.ncis.osdt.itp.upstreaming, kuba, pabeni, davem

Hello:

This series was applied to netdev/net-next.git (main)
by Jakub Kicinski <kuba@kernel.org>:

On Wed, 27 Mar 2024 15:22:38 +0100 you wrote:
> Some structures contain flexible arrays at the end and the counter for
> them, but the counter has explicit Endianness and thus __counted_by()
> can't be used directly.
> 
> To increase test coverage for potential problems without breaking
> anything, introduce __counted_by_{le,be} defined depending on platform's
> Endianness to either __counted_by() when applicable or noop otherwise.
> The first user will be virtchnl2.h from idpf just as example with 9 flex
> structures having Little Endian counters.
> 
> [...]

Here is the summary with links:
  - [net-next,v2,1/3] compiler_types: add Endianness-dependent __counted_by_{le,be}
    https://git.kernel.org/netdev/net-next/c/ca7e324e8ad3
  - [net-next,v2,2/3] idpf: make virtchnl2.h self-contained
    https://git.kernel.org/netdev/net-next/c/c00d33f1fc79
  - [net-next,v2,3/3] idpf: sprinkle __counted_by{,_le}() in the virtchnl2 header
    https://git.kernel.org/netdev/net-next/c/93d24acfa05e

You are awesome, thank you!
-- 
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html



^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH net-next v2 0/3] compiler_types: add Endianness-dependent __counted_by_{le,be}
@ 2024-03-29  2:20   ` patchwork-bot+netdevbpf
  0 siblings, 0 replies; 12+ messages in thread
From: patchwork-bot+netdevbpf @ 2024-03-29  2:20 UTC (permalink / raw)
  To: Alexander Lobakin
  Cc: davem, edumazet, kuba, pabeni, keescook, gustavoars, nathan,
	horms, nex.sw.ncis.osdt.itp.upstreaming, intel-wired-lan,
	linux-hardening, netdev, linux-kernel

Hello:

This series was applied to netdev/net-next.git (main)
by Jakub Kicinski <kuba@kernel.org>:

On Wed, 27 Mar 2024 15:22:38 +0100 you wrote:
> Some structures contain flexible arrays at the end and the counter for
> them, but the counter has explicit Endianness and thus __counted_by()
> can't be used directly.
> 
> To increase test coverage for potential problems without breaking
> anything, introduce __counted_by_{le,be} defined depending on platform's
> Endianness to either __counted_by() when applicable or noop otherwise.
> The first user will be virtchnl2.h from idpf just as example with 9 flex
> structures having Little Endian counters.
> 
> [...]

Here is the summary with links:
  - [net-next,v2,1/3] compiler_types: add Endianness-dependent __counted_by_{le,be}
    https://git.kernel.org/netdev/net-next/c/ca7e324e8ad3
  - [net-next,v2,2/3] idpf: make virtchnl2.h self-contained
    https://git.kernel.org/netdev/net-next/c/c00d33f1fc79
  - [net-next,v2,3/3] idpf: sprinkle __counted_by{,_le}() in the virtchnl2 header
    https://git.kernel.org/netdev/net-next/c/93d24acfa05e

You are awesome, thank you!
-- 
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html



^ permalink raw reply	[flat|nested] 12+ messages in thread

end of thread, other threads:[~2024-03-29  2:20 UTC | newest]

Thread overview: 12+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2024-03-27 14:22 [PATCH net-next v2 0/3] compiler_types: add Endianness-dependent __counted_by_{le,be} Alexander Lobakin
2024-03-27 14:22 ` [Intel-wired-lan] [PATCH net-next v2 0/3] compiler_types: add Endianness-dependent __counted_by_{le, be} Alexander Lobakin
2024-03-27 14:22 ` [PATCH net-next v2 1/3] compiler_types: add Endianness-dependent __counted_by_{le,be} Alexander Lobakin
2024-03-27 14:22   ` [Intel-wired-lan] [PATCH net-next v2 1/3] compiler_types: add Endianness-dependent __counted_by_{le, be} Alexander Lobakin
2024-03-27 14:22 ` [PATCH net-next v2 2/3] idpf: make virtchnl2.h self-contained Alexander Lobakin
2024-03-27 14:22   ` [Intel-wired-lan] " Alexander Lobakin
2024-03-27 14:22 ` [PATCH net-next v2 3/3] idpf: sprinkle __counted_by{,_le}() in the virtchnl2 header Alexander Lobakin
2024-03-27 14:22   ` [Intel-wired-lan] [PATCH net-next v2 3/3] idpf: sprinkle __counted_by{, _le}() " Alexander Lobakin
2024-03-28 10:11 ` [PATCH net-next v2 0/3] compiler_types: add Endianness-dependent __counted_by_{le,be} Przemek Kitszel
2024-03-28 10:11   ` [Intel-wired-lan] [PATCH net-next v2 0/3] compiler_types: add Endianness-dependent __counted_by_{le, be} Przemek Kitszel
2024-03-29  2:20 ` patchwork-bot+netdevbpf
2024-03-29  2:20   ` [PATCH net-next v2 0/3] compiler_types: add Endianness-dependent __counted_by_{le,be} patchwork-bot+netdevbpf

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.