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 Cc: linux-r...@vger.kernel.org Signed-off-by: Kees Cook Reviewed-by: Gustavo A. R. Silva 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_tarray_sectors; int raid_disks; /* a copy of mddev->raid_disks */ - struct dev_info disks[]; + struct dev_info disks[] __counted_by(raid_disks); }; #endif
[PATCH] md/md-linear: Annotate struct linear_conf with __counted_by
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 Cc: linux-r...@vger.kernel.org Signed-off-by: Kees Cook --- 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_tarray_sectors; int raid_disks; /* a copy of mddev->raid_disks */ - struct dev_info disks[]; + struct dev_info disks[] __counted_by(raid_disks); }; #endif -- 2.34.1
