| Message ID | 20230915200328.never.064-kees@kernel.org (mailing list archive) |
|---|---|
| State | Accepted, archived |
| Headers | show |
| Series | md/md-linear: Annotate struct linear_conf with __counted_by | expand |
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 <song@kernel.org> > Cc: linux-raid@vger.kernel.org > Signed-off-by: Kees Cook <keescook@chromium.org> Reviewed-by: Gustavo A. R. Silva <gustavoars@kernel.org> Thanks
On Fri, Sep 15, 2023 at 1:27 PM Gustavo A. R. Silva <gustavo@embeddedor.com> 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 <song@kernel.org> > > Cc: linux-raid@vger.kernel.org > > Signed-off-by: Kees Cook <keescook@chromium.org> > > Reviewed-by: Gustavo A. R. Silva <gustavoars@kernel.org> Applied to md-next. Thanks! Song
On Fri, 15 Sep 2023 13:03:28 -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 linear_conf. > 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] md/md-linear: Annotate struct linear_conf with __counted_by https://git.kernel.org/kees/c/9add7681e09b Take care,
On Fri, Sep 29, 2023 at 12:21 PM Kees Cook <keescook@chromium.org> wrote: > > On Fri, 15 Sep 2023 13:03:28 -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 linear_conf. > > 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] md/md-linear: Annotate struct linear_conf with __counted_by > https://git.kernel.org/kees/c/9add7681e09b Hmm.. Jens pulled this into his for-next branch and for-6.7/block branch earlier today: https://git.kernel.org/pub/scm/linux/kernel/git/axboe/linux-block.git/commit/?h=for-next&id=e887544d7620f1d3cef017e45df7bc625182caff Do we need to do anything about this (drop one of them)? Thanks, Song
On Fri, Sep 29, 2023 at 04:40:13PM -0700, Song Liu wrote: > On Fri, Sep 29, 2023 at 12:21 PM Kees Cook <keescook@chromium.org> wrote: > > > > On Fri, 15 Sep 2023 13:03:28 -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 linear_conf. > > > 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] md/md-linear: Annotate struct linear_conf with __counted_by > > https://git.kernel.org/kees/c/9add7681e09b > > Hmm.. > > Jens pulled this into his for-next branch and for-6.7/block branch > earlier today: > > https://git.kernel.org/pub/scm/linux/kernel/git/axboe/linux-block.git/commit/?h=for-next&id=e887544d7620f1d3cef017e45df7bc625182caff > > Do we need to do anything about this (drop one of them)? Whoops! Sorry, I hadn't seen it picked up. I'll drop it from my tree. Thanks! -Kees
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
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 <song@kernel.org> Cc: linux-raid@vger.kernel.org Signed-off-by: Kees Cook <keescook@chromium.org> --- drivers/md/md-linear.c | 26 +++++++++++++------------- drivers/md/md-linear.h | 2 +- 2 files changed, 14 insertions(+), 14 deletions(-)