2023-09-15 20:41:04

by Gustavo A. R. Silva

[permalink] [raw]
Subject: Re: [PATCH] md/md-linear: Annotate struct linear_conf with __counted_by



On 9/15/23 14:03, 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 linear_conf.
> 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: Song Liu <[email protected]>
> Cc: [email protected]
> Signed-off-by: Kees Cook <[email protected]>

Reviewed-by: Gustavo A. R. Silva <[email protected]>

Thanks
--
Gustavo

> ---
> drivers/md/md-linear.c | 26 +++++++++++++-------------
> drivers/md/md-linear.h | 2 +-
> 2 files changed, 14 insertions(+), 14 deletions(-)
>
> diff --git a/drivers/md/md-linear.c b/drivers/md/md-linear.c
> index 71ac99646827..ae2826e9645b 100644
> --- a/drivers/md/md-linear.c
> +++ b/drivers/md/md-linear.c
> @@ -69,6 +69,19 @@ static struct linear_conf *linear_conf(struct mddev *mddev, int raid_disks)
> if (!conf)
> return NULL;
>
> + /*
> + * conf->raid_disks is copy of mddev->raid_disks. The reason to
> + * keep a copy of mddev->raid_disks in struct linear_conf is,
> + * mddev->raid_disks may not be consistent with pointers number of
> + * conf->disks[] when it is updated in linear_add() and used to
> + * iterate old conf->disks[] earray in linear_congested().
> + * Here conf->raid_disks is always consitent with number of
> + * pointers in conf->disks[] array, and mddev->private is updated
> + * with rcu_assign_pointer() in linear_addr(), such race can be
> + * avoided.
> + */
> + conf->raid_disks = raid_disks;
> +
> cnt = 0;
> conf->array_sectors = 0;
>
> @@ -112,19 +125,6 @@ static struct linear_conf *linear_conf(struct mddev *mddev, int raid_disks)
> conf->disks[i-1].end_sector +
> conf->disks[i].rdev->sectors;
>
> - /*
> - * conf->raid_disks is copy of mddev->raid_disks. The reason to
> - * keep a copy of mddev->raid_disks in struct linear_conf is,
> - * mddev->raid_disks may not be consistent with pointers number of
> - * conf->disks[] when it is updated in linear_add() and used to
> - * iterate old conf->disks[] earray in linear_congested().
> - * Here conf->raid_disks is always consitent with number of
> - * pointers in conf->disks[] array, and mddev->private is updated
> - * with rcu_assign_pointer() in linear_addr(), such race can be
> - * avoided.
> - */
> - conf->raid_disks = raid_disks;
> -
> return conf;
>
> out:
> diff --git a/drivers/md/md-linear.h b/drivers/md/md-linear.h
> index 24e97db50ebb..5587eeedb882 100644
> --- a/drivers/md/md-linear.h
> +++ b/drivers/md/md-linear.h
> @@ -12,6 +12,6 @@ struct linear_conf
> struct rcu_head rcu;
> sector_t array_sectors;
> int raid_disks; /* a copy of mddev->raid_disks */
> - struct dev_info disks[];
> + struct dev_info disks[] __counted_by(raid_disks);
> };
> #endif


2023-09-22 18:39:06

by Song Liu

[permalink] [raw]
Subject: Re: [PATCH] md/md-linear: Annotate struct linear_conf with __counted_by

On Fri, Sep 15, 2023 at 1:27 PM Gustavo A. R. Silva
<[email protected]> wrote:
>
>
>
> On 9/15/23 14:03, 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 linear_conf.
> > 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: Song Liu <[email protected]>
> > Cc: [email protected]
> > Signed-off-by: Kees Cook <[email protected]>
>
> Reviewed-by: Gustavo A. R. Silva <[email protected]>

Applied to md-next. Thanks!
Song