Skip to contentSkip to conversation

hte: Annotate struct hte_device with __counted_by

PATCH In review Kees Cook 4 participants 4 messages Sep 22, 2023 – Sep 28, 2023 HTE

lore
Share & tools
lore

Kees Cook

author
#0 lore raw

PATCH 1/1 hte: Annotate struct hte_device 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 checking 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 hte_device.

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

Cc Dipen Patel <[email protected]>
Cc [email protected]
Signed-off-by Kees Cook <[email protected]>
---
 drivers/hte/hte.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
drivers/hte/hte.c +1 -1
index 598a716b7364..1fd8d2d4528b 100644
@@ -88,7 +88,7 @@ struct hte_device {
 	struct list_head list;
 	struct hte_chip *chip;
 	struct module *owner;
-	struct hte_ts_info ei[];
+	struct hte_ts_info ei[] __counted_by(nlines);
 };
 
 #ifdef CONFIG_DEBUG_FS
Signature · 1 line
2.34.1
Reviewed-by on v1 1/1
#1 lore raw
Quoted 6 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 checking via CONFIG_UBSAN_BOUNDS (for array indexing) and CONFIG_FORTIFY_SOURCE (for strcpy/memcpy-family functions).

Quoted 7 lines

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

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

Cc Dipen Patel <[email protected]>
Cc [email protected]
Signed-off-by Kees Cook <[email protected]>
Reviewed-by Gustavo A. R. Silva <[email protected]>

Thanks

Signature · 1 line
Gustavo
Quoted 17 lines
---
  drivers/hte/hte.c | 2 +-
  1 file changed, 1 insertion(+), 1 deletion(-)
drivers/hte/hte.c +1 -1
index 598a716b7364..1fd8d2d4528b 100644
@@ -88,7 +88,7 @@ struct hte_device {
  	struct list_head list;
  	struct hte_chip *chip;
  	struct module *owner;
-	struct hte_ts_info ei[];
+	struct hte_ts_info ei[] __counted_by(nlines);
  };
 
  #ifdef CONFIG_DEBUG_FS
maintainer Reviewed-by
#2 lore raw
Quoted 13 lines · Gustavo A. R. Silva
Quoted 10 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 checking 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 hte_device.

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

Quoted 7 lines
Quoted 3 lines
Cc Dipen Patel <[email protected]>
Cc [email protected]
Signed-off-by Kees Cook <[email protected]>
Reviewed-by Gustavo A. R. Silva <[email protected]>

Thanks

Reviewed-by Dipen Patel <[email protected]>
Reviewed-by on v1 1/1
#3 lore raw
In reply to Kees Cook
Quoted 7 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 checking via CONFIG_UBSAN_BOUNDS (for array indexing) and CONFIG_FORTIFY_SOURCE (for strcpy/memcpy-family functions).

Quoted 7 lines

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

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

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

Yep, this flexible array member is counted by @nlines which is evident throughout hte.c:

        gdev = kzalloc(struct_size(gdev, ei, chip->nlines), GFP_KERNEL);
        ...
        gdev->nlines = chip->nlines;
        ...
        for (i = 0; i < chip->nlines; i++) {
          gdev->ei[i].gdev = gdev;
        ...
Reviewed-by Justin Stitt <[email protected]>
Quoted 21 lines
---
 drivers/hte/hte.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
drivers/hte/hte.c +1 -2
index 598a716b7364..1fd8d2d4528b 100644
@@ -88,7 +88,7 @@ struct hte_device {
        struct list_head list;
        struct hte_chip *chip;
        struct module *owner;
-       struct hte_ts_info ei[];
+       struct hte_ts_info ei[] __counted_by(nlines);
 };
 
 #ifdef CONFIG_DEBUG_FS
--

2.34.1