Received-SPF: pass (google.com: best guess record for domain of linux-kernel-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) client-ip=209.132.180.67;
MIME-Version: 1.0
In-Reply-To: <20180411044315.GA46144@beast>
References: <20180411044315.GA46144@beast>
From:   Kees Cook <keescook@chromium.org>
Date:   Thu, 26 Apr 2018 16:31:50 -0700
Message-ID: <CAGXu5jLUTF0c3DLGjFFnSwJ3FJmiOMYRu3vqTqNVEBQehyn4KA@mail.gmail.com>
Subject: Re: [PATCH] dm/raid1: Remove VLA usage
To:     Alasdair Kergon <agk@redhat.com>
Cc:     Mike Snitzer <snitzer@redhat.com>,
        LKML <linux-kernel@vger.kernel.org>, dm-devel@redhat.com,
        Tycho Andersen <tycho@tycho.ws>
Content-Type: text/plain; charset="UTF-8"
Sender: linux-kernel-owner@vger.kernel.org
Precedence: bulk

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