Skip to contentSkip to conversation

mailbox: zynqmp: Annotate struct zynqmp_ipi_pdata with __counted_by

PATCH Applied Kees Cook 4 participants 5 messages Sep 22, 2023 – Oct 6, 2023

Subsystems (2)
Arm/Zynq architecture Mailbox API
lore
Share & tools
lore

Kees Cook

author
#0 lore raw

PATCH 1/1 mailbox: zynqmp: Annotate struct zynqmp_ipi_pdata 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 zynqmp_ipi_pdata.

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

Cc Jassi Brar <[email protected]>
Cc Michal Simek <[email protected]>
Cc [email protected]
Signed-off-by Kees Cook <[email protected]>
---
 drivers/mailbox/zynqmp-ipi-mailbox.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
drivers/mailbox/zynqmp-ipi-mailbox.c +1 -1
index e4fcac97dbfa..7fa533e80dd9 100644
@@ -108,7 +108,7 @@ struct zynqmp_ipi_pdata {
 	unsigned int method;
 	u32 local_id;
 	int num_mboxes;
-	struct zynqmp_ipi_mbox ipi_mboxes[];
+	struct zynqmp_ipi_mbox ipi_mboxes[] __counted_by(num_mboxes);
 };
 
 static struct device_driver zynqmp_ipi_mbox_driver = {
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 8 lines

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

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

Cc Jassi Brar <[email protected]>
Cc Michal Simek <[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/mailbox/zynqmp-ipi-mailbox.c | 2 +-
  1 file changed, 1 insertion(+), 1 deletion(-)
drivers/mailbox/zynqmp-ipi-mailbox.c +1 -1
index e4fcac97dbfa..7fa533e80dd9 100644
@@ -108,7 +108,7 @@ struct zynqmp_ipi_pdata {
  	unsigned int method;
  	u32 local_id;
  	int num_mboxes;
-	struct zynqmp_ipi_mbox ipi_mboxes[];
+	struct zynqmp_ipi_mbox ipi_mboxes[] __counted_by(num_mboxes);
  };
 
  static struct device_driver zynqmp_ipi_mbox_driver = {
maintainer Acked-by on v1 1/1
#2 lore raw
In reply to Kees Cook
Quoted 23 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 zynqmp_ipi_pdata.

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

Cc Jassi Brar <[email protected]>
Cc Michal Simek <[email protected]>
Cc [email protected]
Signed-off-by Kees Cook <[email protected]>
---
  drivers/mailbox/zynqmp-ipi-mailbox.c | 2 +-
  1 file changed, 1 insertion(+), 1 deletion(-)
drivers/mailbox/zynqmp-ipi-mailbox.c +0 -0
index e4fcac97dbfa..7fa533e80dd9 100644

drivers/mailbox/zynqmp-ipi-mailbox.c struct zynqmp_ipi_pdata { · line 114

Quoted 8 lines
drivers/mailbox/zynqmp-ipi-mailbox.c +1 -1
@@ -108,7 +108,7 @@ struct zynqmp_ipi_pdata {
  	unsigned int method;
  	u32 local_id;
  	int num_mboxes;
-	struct zynqmp_ipi_mbox ipi_mboxes[];
+	struct zynqmp_ipi_mbox ipi_mboxes[] __counted_by(num_mboxes);
  };
 
  static struct device_driver zynqmp_ipi_mbox_driver = {
Acked-by Michal Simek <[email protected]>

Thanks,
Michal

Reviewed-by on v1 1/1
#3 lore raw
In reply to Kees Cook
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 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 zynqmp_ipi_pdata.

Quoted 7 lines

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

Cc Jassi Brar <[email protected]>
Cc Michal Simek <[email protected]>
Cc [email protected]
Signed-off-by Kees Cook <[email protected]>

---

Great patch! Crucially, the count is _correctly_ assigned to before the flexible array member is accessed.

Reviewed-by Justin Stitt <[email protected]>
Quoted 17 lines
 drivers/mailbox/zynqmp-ipi-mailbox.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
drivers/mailbox/zynqmp-ipi-mailbox.c +1 -2
index e4fcac97dbfa..7fa533e80dd9 100644
@@ -108,7 +108,7 @@ struct zynqmp_ipi_pdata {
        unsigned int method;
        u32 local_id;
        int num_mboxes;
-       struct zynqmp_ipi_mbox ipi_mboxes[];
+       struct zynqmp_ipi_mbox ipi_mboxes[] __counted_by(num_mboxes);
 };
 
 static struct device_driver zynqmp_ipi_mbox_driver = {
--
Quoted 3 lines

2.34.1

Thanks
Justin

author Applied
#4 lore raw
In reply to Kees Cook
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 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 zynqmp_ipi_pdata.

[...]

Applied to for-next/hardening, thanks!

[1/1] mailbox: zynqmp: Annotate struct zynqmp_ipi_pdata with __counted_by

      https://git.kernel.org/kees/c/6b607a3a311a

Take care,

Signature · 1 line
Kees Cook