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@xxxxxxxxxxxxxx>
> Signed-off-by: Kees Cook <keescook@xxxxxxxxxxxx>

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

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;