md/md-linear: Annotate struct linear_conf with __counted_by
PATCH 1/1 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
--- drivers/md/md-linear.c | 26 +++++++++++++------------- drivers/md/md-linear.h | 2 +- 2 files changed, 14 insertions(+), 14 deletions(-)
drivers/md/md-linear.c +13 -13
@@ -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:
drivers/md/md-linear.h +1 -1
@@ -12,6 +12,6 @@ struct linear_confstruct 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
Signature · 1 line
2.34.1
Quoted 10 lines · Kees Cook
Prepare for the coming implementation by GCC and Clang of the
__counted_byattribute. Flexible array members annotated with__counted_bycan have their accesses bounds-checked at run-time checking viaCONFIG_UBSAN_BOUNDS(for array indexing) andCONFIG_FORTIFY_SOURCE(for strcpy/memcpy-family functions).As found with Coccinelle[1], add
__counted_byfor structlinear_conf. Additionally, since the element count member must be set before accessing the annotated flexible array member, move its initialization earlier.
Quoted 5 lines
[1] https://github.com/kees/kernel-tools/blob/trunk/coccinelle/examples/counted_by.cocci
Thanks
Signature · 1 line
Gustavo
Quoted 61 lines
--- drivers/md/md-linear.c | 26 +++++++++++++------------- drivers/md/md-linear.h | 2 +- 2 files changed, 14 insertions(+), 14 deletions(-)drivers/md/md-linear.c +13
-13@@ -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:drivers/md/md-linear.h +1
-1@@ -12,6 +12,6 @@ struct linear_confstruct 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
Quoted 14 lines · Gustavo A. R. Silva
Quoted 10 lines · Kees Cook
Prepare for the coming implementation by GCC and Clang of the
__counted_byattribute. Flexible array members annotated with__counted_bycan have their accesses bounds-checked at run-time checking viaCONFIG_UBSAN_BOUNDS(for array indexing) andCONFIG_FORTIFY_SOURCE(for strcpy/memcpy-family functions).As found with Coccinelle[1], add
__counted_byfor structlinear_conf. Additionally, since the element count member must be set before accessing the annotated flexible array member, move its initialization earlier.
Quoted 7 lines
Quoted 5 lines
[1] https://github.com/kees/kernel-tools/blob/trunk/coccinelle/examples/counted_by.cocci
Reviewed-by Gustavo A. R. Silva <[email protected]>
Applied to md-next. Thanks!
Song
Kees Cook
AppliedQuoted 11 lines · Kees Cook
Prepare for the coming implementation by GCC and Clang of the
__counted_byattribute. Flexible array members annotated with__counted_bycan have their accesses bounds-checked at run-time checking viaCONFIG_UBSAN_BOUNDS(for array indexing) andCONFIG_FORTIFY_SOURCE(for strcpy/memcpy-family functions).As found with Coccinelle[1], add
__counted_byfor structlinear_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,
Signature · 1 line
Kees Cook
Song Liu
maintainerQuoted 12 lines · Kees Cook
Quoted 10 lines · Kees Cook
Prepare for the coming implementation by GCC and Clang of the
__counted_byattribute. Flexible array members annotated with__counted_bycan have their accesses bounds-checked at run-time checking viaCONFIG_UBSAN_BOUNDS(for array indexing) andCONFIG_FORTIFY_SOURCE(for strcpy/memcpy-family functions).As found with Coccinelle[1], add
__counted_byfor structlinear_conf. Additionally, since the element count member must be set before accessing the annotated flexible array member, move its initialization earlier.
Quoted 6 lines
Quoted 1 line
[...]
Applied to for-next/hardening, thanks!
[1/1] md/md-linear: Annotate struct
linear_confwith__counted_byhttps://git.kernel.org/kees/c/9add7681e09b
Hmm..
Jens pulled this into his for-next branch and for-6.7/block branch earlier today:
Do we need to do anything about this (drop one of them)?
Thanks,
Song
Quoted 20 lines · Song Liu
Quoted 12 lines · Kees Cook
Quoted 10 lines · Kees Cook
Prepare for the coming implementation by GCC and Clang of the
__counted_byattribute. Flexible array members annotated with__counted_bycan have their accesses bounds-checked at run-time checking viaCONFIG_UBSAN_BOUNDS(for array indexing) andCONFIG_FORTIFY_SOURCE(for strcpy/memcpy-family functions).As found with Coccinelle[1], add
__counted_byfor structlinear_conf. Additionally, since the element count member must be set before accessing the annotated flexible array member, move its initialization earlier.Quoted 6 lines
Quoted 1 line
[...]
Applied to for-next/hardening, thanks!
[1/1] md/md-linear: Annotate struct
linear_confwith__counted_byhttps://git.kernel.org/kees/c/9add7681e09b
Quoted 8 lines
Hmm..
Jens pulled this into his for-next branch and for-6.7/block branch earlier today:
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
Signature · 1 line
Kees Cook