[PATCH] reset: Annotate struct reset_control_array with __counted_by

[PATCH] reset: Annotate struct reset_control_array with __counted_by

[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 reset_control_array.
Additionally, since the element count member must be set before accessing
the annotated flexible array member, move its initialization earlier.

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

Cc: Philipp Zabel <p.zabel@pengutronix.de>
Signed-off-by: Kees Cook <keescook@chromium.org>
---
 drivers/reset/core.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/reset/core.c b/drivers/reset/core.c
index f0a076e94118..7ece6a8e9858 100644
--- a/drivers/reset/core.c
+++ b/drivers/reset/core.c
@@ -60,7 +60,7 @@ struct reset_control {
 struct reset_control_array {
 	struct reset_control base;
 	unsigned int num_rstcs;
-	struct reset_control *rstc[];
+	struct reset_control *rstc[] __counted_by(num_rstcs);
 };
 
 static const char *rcdev_name(struct reset_controller_dev *rcdev)
@@ -1185,6 +1185,7 @@ of_reset_control_array_get(struct device_node *np, bool shared, bool optional,
 	resets = kzalloc(struct_size(resets, rstc, num), GFP_KERNEL);
 	if (!resets)
 		return ERR_PTR(-ENOMEM);
+	resets->num_rstcs = num;
 
 	for (i = 0; i < num; i++) {
 		rstc = __of_reset_control_get(np, NULL, i, shared, optional,
@@ -1193,7 +1194,6 @@ of_reset_control_array_get(struct device_node *np, bool shared, bool optional,
 			goto err_rst;
 		resets->rstc[i] = rstc;
 	}
-	resets->num_rstcs = num;
 	resets->base.array = true;
 
 	return &resets->base;
-- 
2.34.1

Re: [PATCH] reset: Annotate struct reset_control_array with __counted_by

[Gustavo A. R. Silva]

On 9/22/23 11:52, Kees Cook wrote:
> 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 reset_control_array.
> Additionally, since the element count member must be set before accessing
> the annotated flexible array member, move its initialization earlier.
> 
> [1] https://github.com/kees/kernel-tools/blob/trunk/coccinelle/examples/counted_by.cocci
> 
> Cc: Philipp Zabel <p.zabel@pengutronix.de>
> Signed-off-by: Kees Cook <keescook@chromium.org>

Reviewed-by: Gustavo A. R. Silva <gustavoars@kernel.org>

Thanks
-- 
Gustavo

> ---
>   drivers/reset/core.c | 4 ++--
>   1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/reset/core.c b/drivers/reset/core.c
> index f0a076e94118..7ece6a8e9858 100644
> --- a/drivers/reset/core.c
> +++ b/drivers/reset/core.c
> @@ -60,7 +60,7 @@ struct reset_control {
>   struct reset_control_array {
>   	struct reset_control base;
>   	unsigned int num_rstcs;
> -	struct reset_control *rstc[];
> +	struct reset_control *rstc[] __counted_by(num_rstcs);
>   };
>   
>   static const char *rcdev_name(struct reset_controller_dev *rcdev)
> @@ -1185,6 +1185,7 @@ of_reset_control_array_get(struct device_node *np, bool shared, bool optional,
>   	resets = kzalloc(struct_size(resets, rstc, num), GFP_KERNEL);
>   	if (!resets)
>   		return ERR_PTR(-ENOMEM);
> +	resets->num_rstcs = num;
>   
>   	for (i = 0; i < num; i++) {
>   		rstc = __of_reset_control_get(np, NULL, i, shared, optional,
> @@ -1193,7 +1194,6 @@ of_reset_control_array_get(struct device_node *np, bool shared, bool optional,
>   			goto err_rst;
>   		resets->rstc[i] = rstc;
>   	}
> -	resets->num_rstcs = num;
>   	resets->base.array = true;
>   
>   	return &resets->base;

Re: [PATCH] reset: Annotate struct reset_control_array with __counted_by

[Kees Cook]

On Fri, 22 Sep 2023 10:52:29 -0700, Kees Cook wrote:
> 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 reset_control_array.
> Additionally, since the element count member must be set before accessing
> the annotated flexible array member, move its initialization earlier.
> 
> [...]

Applied to for-next/hardening, thanks!

[1/1] reset: Annotate struct reset_control_array with __counted_by
      https://git.kernel.org/kees/c/fed2ef7abaeb

Take care,

-- 
Kees Cook