* [PATCH] md/md-linear: Annotate struct linear_conf with __counted_by
@ 2023-09-15 20:03 Kees Cook
2023-09-15 20:28 ` Gustavo A. R. Silva
2023-09-29 19:21 ` Kees Cook
0 siblings, 2 replies; 6+ messages in thread
From: Kees Cook @ 2023-09-15 20:03 UTC (permalink / raw)
To: Song Liu
Cc: Kees Cook, linux-raid, Nathan Chancellor, Nick Desaulniers,
Tom Rix, linux-kernel, llvm, linux-hardening
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(-)
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
--
2.34.1
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH] md/md-linear: Annotate struct linear_conf with __counted_by
2023-09-15 20:03 [PATCH] md/md-linear: Annotate struct linear_conf with __counted_by Kees Cook
@ 2023-09-15 20:28 ` Gustavo A. R. Silva
2023-09-22 18:27 ` Song Liu
2023-09-29 19:21 ` Kees Cook
1 sibling, 1 reply; 6+ messages in thread
From: Gustavo A. R. Silva @ 2023-09-15 20:28 UTC (permalink / raw)
To: Kees Cook, Song Liu
Cc: linux-raid, Nathan Chancellor, Nick Desaulniers, Tom Rix,
linux-kernel, llvm, linux-hardening
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
--
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
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] md/md-linear: Annotate struct linear_conf with __counted_by
2023-09-15 20:28 ` Gustavo A. R. Silva
@ 2023-09-22 18:27 ` Song Liu
0 siblings, 0 replies; 6+ messages in thread
From: Song Liu @ 2023-09-22 18:27 UTC (permalink / raw)
To: Gustavo A. R. Silva
Cc: Kees Cook, linux-raid, Nathan Chancellor, Nick Desaulniers,
Tom Rix, linux-kernel, llvm, linux-hardening
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
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] md/md-linear: Annotate struct linear_conf with __counted_by
2023-09-15 20:03 [PATCH] md/md-linear: Annotate struct linear_conf with __counted_by Kees Cook
2023-09-15 20:28 ` Gustavo A. R. Silva
@ 2023-09-29 19:21 ` Kees Cook
2023-09-29 23:40 ` Song Liu
1 sibling, 1 reply; 6+ messages in thread
From: Kees Cook @ 2023-09-29 19:21 UTC (permalink / raw)
To: Song Liu, Kees Cook
Cc: linux-raid, Nathan Chancellor, Nick Desaulniers, Tom Rix,
linux-kernel, llvm, linux-hardening
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,
--
Kees Cook
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] md/md-linear: Annotate struct linear_conf with __counted_by
2023-09-29 19:21 ` Kees Cook
@ 2023-09-29 23:40 ` Song Liu
2023-09-29 23:43 ` Kees Cook
0 siblings, 1 reply; 6+ messages in thread
From: Song Liu @ 2023-09-29 23:40 UTC (permalink / raw)
To: Kees Cook, Jens Axboe
Cc: linux-raid, Nathan Chancellor, Nick Desaulniers, Tom Rix,
linux-kernel, llvm, linux-hardening
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
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] md/md-linear: Annotate struct linear_conf with __counted_by
2023-09-29 23:40 ` Song Liu
@ 2023-09-29 23:43 ` Kees Cook
0 siblings, 0 replies; 6+ messages in thread
From: Kees Cook @ 2023-09-29 23:43 UTC (permalink / raw)
To: Song Liu
Cc: Jens Axboe, linux-raid, Nathan Chancellor, Nick Desaulniers,
Tom Rix, linux-kernel, llvm, linux-hardening
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
--
Kees Cook
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2023-09-29 23:43 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2023-09-15 20:03 [PATCH] md/md-linear: Annotate struct linear_conf with __counted_by Kees Cook
2023-09-15 20:28 ` Gustavo A. R. Silva
2023-09-22 18:27 ` Song Liu
2023-09-29 19:21 ` Kees Cook
2023-09-29 23:40 ` Song Liu
2023-09-29 23:43 ` Kees Cook
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).