linux-kernel - Re: [PATCH] reset: Annotate struct reset_control_array with __counted

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <fd9a945e-b5e6-2926-af22-e632ad2281c6@embeddedor.com>
Date:   Sat, 23 Sep 2023 10:35:46 -0600
From:   "Gustavo A. R. Silva" <gustavo@...eddedor.com>
To:     Kees Cook <keescook@...omium.org>,
        Philipp Zabel <p.zabel@...gutronix.de>
Cc:     Nathan Chancellor <nathan@...nel.org>,
        Nick Desaulniers <ndesaulniers@...gle.com>,
        Tom Rix <trix@...hat.com>, linux-kernel@...r.kernel.org,
        llvm@...ts.linux.dev, linux-hardening@...r.kernel.org
Subject: Re: [PATCH] reset: Annotate struct reset_control_array with
 __counted_by



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@...gutronix.de>
> Signed-off-by: Kees Cook <keescook@...omium.org>

Reviewed-by: Gustavo A. R. Silva <gustavoars@...nel.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;