net/mlx5: Annotate struct mlx5_fc_bulk with __counted_by

PATCH Applied Kees Cook 5 participants 5 messages Oct 3, 2023 – Oct 12, 2023

Subsystems (4)

Networking Network drivers InfiniBand Mellanox mlx5 core

lore

Share & tools

lore

Kees Cook

author

2023-10-03 23:17 UTC #0 lore raw

PATCH 1/1 net/mlx5: Annotate struct mlx5_fc_bulk with __counted_by

Prepare for the coming implementation by GCC and Clang of the __counted_by attribute. Flexible array members annotated with __counted_by can have their accesses bounds-checked at run-time via CONFIG_UBSAN_BOUNDS (for array indexing) and CONFIG_FORTIFY_SOURCE (for strcpy/memcpy-family functions).

As found with Coccinelle[1], add __counted_by for struct mlx5_fc_bulk.

Cc Saeed Mahameed <[email protected]>

Cc Leon Romanovsky <[email protected]>

Cc "David S. Miller" <[email protected]>

Cc Eric Dumazet <[email protected]>

Cc Jakub Kicinski <[email protected]>

Cc Paolo Abeni <[email protected]>

Cc [email protected]

Link https://github.com/kees/kernel-tools/blob/trunk/coccinelle/examples/counted_by.cocci [1]

Signed-off-by Kees Cook <[email protected]>

---
 drivers/net/ethernet/mellanox/mlx5/core/fs_counters.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

drivers/net/ethernet/mellanox/mlx5/core/fs_counters.c +1 -1

index 17fe30a4c06c..0c26d707eed2 100644
@@ -539,7 +539,7 @@ struct mlx5_fc_bulk {
 	u32 base_id;
 	int bulk_len;
 	unsigned long *bitmask;
-	struct mlx5_fc fcs[];
+	struct mlx5_fc fcs[] __counted_by(bulk_len);
 };
 
 static void mlx5_fc_init(struct mlx5_fc *counter, struct mlx5_fc_bulk *bulk,

Signature · 1 line

2.34.1

Justin Stitt

Reviewed-by on v1 1/1

2023-10-03 23:21 UTC #1 lore raw

Quoted 28 lines · Kees Cook

Prepare for the coming implementation by GCC and Clang of the __counted_by attribute. Flexible array members annotated with __counted_by can have their accesses bounds-checked at run-time via CONFIG_UBSAN_BOUNDS (for array indexing) and CONFIG_FORTIFY_SOURCE (for strcpy/memcpy-family functions).
As found with Coccinelle[1], add __counted_by for struct mlx5_fc_bulk.
Cc Saeed Mahameed <[email protected]>
Cc Leon Romanovsky <[email protected]>
Cc "David S. Miller" <[email protected]>
Cc Eric Dumazet <[email protected]>
Cc Jakub Kicinski <[email protected]>
Cc Paolo Abeni <[email protected]>
Cc [email protected]
Cc [email protected]
Link https://github.com/kees/kernel-tools/blob/trunk/coccinelle/examples/counted_by.cocci [1]
Signed-off-by Kees Cook <[email protected]>
---
 drivers/net/ethernet/mellanox/mlx5/core/fs_counters.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
drivers/net/ethernet/mellanox/mlx5/core/fs_counters.c +0 -0
index 17fe30a4c06c..0c26d707eed2 100644

drivers/net/ethernet/mellanox/mlx5/core/fs_counters.c struct mlx5_fc_bulk { · line 543

Quoted 6 lines

drivers/net/ethernet/mellanox/mlx5/core/fs_counters.c +1 -1

@@ -539,7 +539,7 @@ struct mlx5_fc_bulk {
        u32 base_id;
        int bulk_len;
        unsigned long *bitmask;
-       struct mlx5_fc fcs[];
+       struct mlx5_fc fcs[] __counted_by(bulk_len);
 };

This looks good.

bulk->bulk_len is assigned before flexible array member fcs is accessed.

        bulk->bulk_len = bulk_len;
        for (i = 0; i < bulk_len; i++) {
                mlx5_fc_init(&bulk->fcs[i], bulk, base_id + i);
                set_bit(i, bulk->bitmask);
        }

Reviewed-by Justin Stitt <[email protected]>

Quoted 6 lines

drivers/net/ethernet/mellanox/mlx5/core/fs_counters.c +0 -1
 static void mlx5_fc_init(struct mlx5_fc *counter, struct mlx5_fc_bulk *bulk,
--
2.34.1

Thanks
Justin

Leon Romanovsky

Reviewed-by

2023-10-04 18:44 UTC #2 lore raw

Quoted 10 lines · Justin Stitt

Quoted 9 lines · Kees Cook
Prepare for the coming implementation by GCC and Clang of the __counted_by attribute. Flexible array members annotated with __counted_by can have their accesses bounds-checked at run-time via CONFIG_UBSAN_BOUNDS (for array indexing) and CONFIG_FORTIFY_SOURCE (for strcpy/memcpy-family functions).
As found with Coccinelle[1], add __counted_by for struct mlx5_fc_bulk.

Quoted 14 lines

Quoted 10 lines
Cc Saeed Mahameed <[email protected]>
Cc Leon Romanovsky <[email protected]>
Cc "David S. Miller" <[email protected]>
Cc Eric Dumazet <[email protected]>
Cc Jakub Kicinski <[email protected]>
Cc Paolo Abeni <[email protected]>
Cc [email protected]
Cc [email protected]
Link https://github.com/kees/kernel-tools/blob/trunk/coccinelle/examples/counted_by.cocci [1]
Signed-off-by Kees Cook <[email protected]>
Quoted 4 lines
---
 drivers/net/ethernet/mellanox/mlx5/core/fs_counters.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

Thanks,

Reviewed-by Leon Romanovsky <[email protected]>

Gustavo A. R. Silva

Reviewed-by on v1 1/1

2023-10-03 23:33 UTC #3 lore raw

In reply to Kees Cook

Quoted 8 lines · Kees Cook

Prepare for the coming implementation by GCC and Clang of the __counted_by attribute. Flexible array members annotated with __counted_by can have their accesses bounds-checked at run-time via CONFIG_UBSAN_BOUNDS (for array indexing) and CONFIG_FORTIFY_SOURCE (for strcpy/memcpy-family functions).
As found with Coccinelle[1], add __counted_by for struct mlx5_fc_bulk.

Quoted 10 lines

Cc Saeed Mahameed <[email protected]>
Cc Leon Romanovsky <[email protected]>
Cc "David S. Miller" <[email protected]>
Cc Eric Dumazet <[email protected]>
Cc Jakub Kicinski <[email protected]>
Cc Paolo Abeni <[email protected]>
Cc [email protected]
Cc [email protected]
Link https://github.com/kees/kernel-tools/blob/trunk/coccinelle/examples/counted_by.cocci [1]
Signed-off-by Kees Cook <[email protected]>

Reviewed-by Gustavo A. R. Silva <[email protected]>

Thanks
--
Gustavo

Quoted 17 lines

---
  drivers/net/ethernet/mellanox/mlx5/core/fs_counters.c | 2 +-
  1 file changed, 1 insertion(+), 1 deletion(-)

drivers/net/ethernet/mellanox/mlx5/core/fs_counters.c +1 -1

index 17fe30a4c06c..0c26d707eed2 100644
@@ -539,7 +539,7 @@ struct mlx5_fc_bulk {
  	u32 base_id;
  	int bulk_len;
  	unsigned long *bitmask;
-	struct mlx5_fc fcs[];
+	struct mlx5_fc fcs[] __counted_by(bulk_len);
  };
 
  static void mlx5_fc_init(struct mlx5_fc *counter, struct mlx5_fc_bulk *bulk,

Saeed Mahameed

Applied

2023-10-12 19:48 UTC #4 lore raw

In reply to Kees Cook

Quoted 8 lines · Kees Cook

Prepare for the coming implementation by GCC and Clang of the __counted_by attribute. Flexible array members annotated with __counted_by can have their accesses bounds-checked at run-time via CONFIG_UBSAN_BOUNDS (for array indexing) and CONFIG_FORTIFY_SOURCE (for strcpy/memcpy-family functions).
As found with Coccinelle[1], add __counted_by for struct mlx5_fc_bulk.

Quoted 10 lines

Cc Saeed Mahameed <[email protected]>
Cc Leon Romanovsky <[email protected]>
Cc "David S. Miller" <[email protected]>
Cc Eric Dumazet <[email protected]>
Cc Jakub Kicinski <[email protected]>
Cc Paolo Abeni <[email protected]>
Cc [email protected]
Cc [email protected]
Link https://github.com/kees/kernel-tools/blob/trunk/coccinelle/examples/counted_by.cocci [1]
Signed-off-by Kees Cook <[email protected]>

Applied to net-next-mlx5.