[PATCH] dm/raid1: Remove VLA usage

[PATCH] dm/raid1: Remove VLA usage

[Kees Cook]

On the quest to remove all VLAs from the kernel[1], this avoids VLAs
in dm-raid1.c by just using the maximum size for the stack arrays.
The nr_mirrors value was already capped at 9, so this makes it a trivial
adjustment to the array sizes.

[1] https://lkml.org/lkml/2018/3/7/621

Signed-off-by: Kees Cook <keescook@chromium.org>
---
 drivers/md/dm-raid1.c | 10 ++++++----
 1 file changed, 6 insertions(+), 4 deletions(-)

diff --git a/drivers/md/dm-raid1.c b/drivers/md/dm-raid1.c
index 580c49cc8079..5903e492bb34 100644
--- a/drivers/md/dm-raid1.c
+++ b/drivers/md/dm-raid1.c
@@ -23,6 +23,8 @@
 
 #define MAX_RECOVERY 1	/* Maximum number of regions recovered in parallel. */
 
+#define MAX_NR_MIRRORS	(DM_KCOPYD_MAX_REGIONS + 1)
+
 #define DM_RAID1_HANDLE_ERRORS	0x01
 #define DM_RAID1_KEEP_LOG	0x02
 #define errors_handled(p)	((p)->features & DM_RAID1_HANDLE_ERRORS)
@@ -255,7 +257,7 @@ static int mirror_flush(struct dm_target *ti)
 	unsigned long error_bits;
 
 	unsigned int i;
-	struct dm_io_region io[ms->nr_mirrors];
+	struct dm_io_region io[MAX_NR_MIRRORS];
 	struct mirror *m;
 	struct dm_io_request io_req = {
 		.bi_op = REQ_OP_WRITE,
@@ -651,7 +653,7 @@ static void write_callback(unsigned long error, void *context)
 static void do_write(struct mirror_set *ms, struct bio *bio)
 {
 	unsigned int i;
-	struct dm_io_region io[ms->nr_mirrors], *dest = io;
+	struct dm_io_region io[MAX_NR_MIRRORS], *dest = io;
 	struct mirror *m;
 	struct dm_io_request io_req = {
 		.bi_op = REQ_OP_WRITE,
@@ -1083,7 +1085,7 @@ static int mirror_ctr(struct dm_target *ti, unsigned int argc, char **argv)
 	argc -= args_used;
 
 	if (!argc || sscanf(argv[0], "%u%c", &nr_mirrors, &dummy) != 1 ||
-	    nr_mirrors < 2 || nr_mirrors > DM_KCOPYD_MAX_REGIONS + 1) {
+	    nr_mirrors < 2 || nr_mirrors > MAX_NR_MIRRORS) {
 		ti->error = "Invalid number of mirrors";
 		dm_dirty_log_destroy(dl);
 		return -EINVAL;
@@ -1404,7 +1406,7 @@ static void mirror_status(struct dm_target *ti, status_type_t type,
 	int num_feature_args = 0;
 	struct mirror_set *ms = (struct mirror_set *) ti->private;
 	struct dm_dirty_log *log = dm_rh_dirty_log(ms->rh);
-	char buffer[ms->nr_mirrors + 1];
+	char buffer[MAX_NR_MIRRORS + 1];
 
 	switch (type) {
 	case STATUSTYPE_INFO:
-- 
2.7.4


-- 
Kees Cook
Pixel Security

Re: [PATCH] dm/raid1: Remove VLA usage

[Kees Cook]

On Tue, Apr 10, 2018 at 9:43 PM, Kees Cook <keescook@chromium.org> wrote:
> On the quest to remove all VLAs from the kernel[1], this avoids VLAs
> in dm-raid1.c by just using the maximum size for the stack arrays.
> The nr_mirrors value was already capped at 9, so this makes it a trivial
> adjustment to the array sizes.
>
> [1] https://lkml.org/lkml/2018/3/7/621
>
> Signed-off-by: Kees Cook <keescook@chromium.org>

Friendly ping... can someone take this, or should this go via another tree?

Thanks!

-Kees

> ---
>  drivers/md/dm-raid1.c | 10 ++++++----
>  1 file changed, 6 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/md/dm-raid1.c b/drivers/md/dm-raid1.c
> index 580c49cc8079..5903e492bb34 100644
> --- a/drivers/md/dm-raid1.c
> +++ b/drivers/md/dm-raid1.c
> @@ -23,6 +23,8 @@
>
>  #define MAX_RECOVERY 1 /* Maximum number of regions recovered in parallel. */
>
> +#define MAX_NR_MIRRORS (DM_KCOPYD_MAX_REGIONS + 1)
> +
>  #define DM_RAID1_HANDLE_ERRORS 0x01
>  #define DM_RAID1_KEEP_LOG      0x02
>  #define errors_handled(p)      ((p)->features & DM_RAID1_HANDLE_ERRORS)
> @@ -255,7 +257,7 @@ static int mirror_flush(struct dm_target *ti)
>         unsigned long error_bits;
>
>         unsigned int i;
> -       struct dm_io_region io[ms->nr_mirrors];
> +       struct dm_io_region io[MAX_NR_MIRRORS];
>         struct mirror *m;
>         struct dm_io_request io_req = {
>                 .bi_op = REQ_OP_WRITE,
> @@ -651,7 +653,7 @@ static void write_callback(unsigned long error, void *context)
>  static void do_write(struct mirror_set *ms, struct bio *bio)
>  {
>         unsigned int i;
> -       struct dm_io_region io[ms->nr_mirrors], *dest = io;
> +       struct dm_io_region io[MAX_NR_MIRRORS], *dest = io;
>         struct mirror *m;
>         struct dm_io_request io_req = {
>                 .bi_op = REQ_OP_WRITE,
> @@ -1083,7 +1085,7 @@ static int mirror_ctr(struct dm_target *ti, unsigned int argc, char **argv)
>         argc -= args_used;
>
>         if (!argc || sscanf(argv[0], "%u%c", &nr_mirrors, &dummy) != 1 ||
> -           nr_mirrors < 2 || nr_mirrors > DM_KCOPYD_MAX_REGIONS + 1) {
> +           nr_mirrors < 2 || nr_mirrors > MAX_NR_MIRRORS) {
>                 ti->error = "Invalid number of mirrors";
>                 dm_dirty_log_destroy(dl);
>                 return -EINVAL;
> @@ -1404,7 +1406,7 @@ static void mirror_status(struct dm_target *ti, status_type_t type,
>         int num_feature_args = 0;
>         struct mirror_set *ms = (struct mirror_set *) ti->private;
>         struct dm_dirty_log *log = dm_rh_dirty_log(ms->rh);
> -       char buffer[ms->nr_mirrors + 1];
> +       char buffer[MAX_NR_MIRRORS + 1];
>
>         switch (type) {
>         case STATUSTYPE_INFO:
> --
> 2.7.4
>
>
> --
> Kees Cook
> Pixel Security



-- 
Kees Cook
Pixel Security

Re: dm/raid1: Remove VLA usage

[Mike Snitzer]

On Thu, Apr 26 2018 at  7:31pm -0400,
Kees Cook <keescook@chromium.org> wrote:

> On Tue, Apr 10, 2018 at 9:43 PM, Kees Cook <keescook@chromium.org> wrote:
> > On the quest to remove all VLAs from the kernel[1], this avoids VLAs
> > in dm-raid1.c by just using the maximum size for the stack arrays.
> > The nr_mirrors value was already capped at 9, so this makes it a trivial
> > adjustment to the array sizes.
> >
> > [1] https://lkml.org/lkml/2018/3/7/621
> >
> > Signed-off-by: Kees Cook <keescook@chromium.org>
> 
> Friendly ping... can someone take this, or should this go via another tree?

It needs further review but it is on my radar.

But please consider it claimed and proceeding upstream for 4.18 via
linux-dm.git

Thanks,
Mike