linux-kernel - Re: [RFC] crypto: ccree - protect against short scatterlists

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <CAMuHMdXHgMn8L2_CZ8kXcp3g4Y+3HfQsvFhyTZatf8-xk2kUdQ@mail.gmail.com>
Date:   Mon, 27 Jan 2020 09:03:19 +0100
From:   Geert Uytterhoeven <geert@...ux-m68k.org>
To:     Gilad Ben-Yossef <gilad@...yossef.com>
Cc:     Herbert Xu <herbert@...dor.apana.org.au>,
        "David S. Miller" <davem@...emloft.net>,
        Ofir Drang <ofir.drang@....com>,
        Linux Crypto Mailing List <linux-crypto@...r.kernel.org>,
        Linux Kernel Mailing List <linux-kernel@...r.kernel.org>
Subject: Re: [RFC] crypto: ccree - protect against short scatterlists

Hi Gilad,

On Sun, Jan 26, 2020 at 2:38 PM Gilad Ben-Yossef <gilad@...yossef.com> wrote:
> Deal gracefully with the event of being handed a scatterlist
> which is shorter than expected.
>
> This mitigates a crash in some cases of crashes due to
> attempt to map empty (but not NULL) scatterlists with none
> zero lengths.
>
> This is an interim patch, to help diagnoze the issue, not
> intended for mainline in its current form as of yet.
>
> Signed-off-by: Gilad Ben-Yossef <gilad@...yossef.com>
> Reported-by: Geert Uytterhoeven <geert@...ux-m68k.org>

Thanks for your patch!

Unfortunately this doesn't make a difference, as ...

> --- a/drivers/crypto/ccree/cc_buffer_mgr.c
> +++ b/drivers/crypto/ccree/cc_buffer_mgr.c
> @@ -286,10 +286,32 @@ static void cc_add_sg_entry(struct device *dev, struct buffer_array *sgl_data,
>         sgl_data->num_of_buffers++;
>  }
>
> +static unsigned int cc_sg_trunc_len(struct scatterlist *sg, unsigned int len)
> +{
> +       unsigned int total;
> +
> +       if (!len)
> +               return 0;
> +
> +       for (total = 0; sg; sg = sg_next(sg)) {
> +               total += sg->length;
> +               if (total >= len) {
> +                       total = len;
> +                       break;
> +               }
> +       }
> +
> +       return total;
> +}
> +
>  static int cc_map_sg(struct device *dev, struct scatterlist *sg,
>                      unsigned int nbytes, int direction, u32 *nents,
>                      u32 max_sg_nents, u32 *lbytes, u32 *mapped_nents)
>  {
> +       int ret;
> +
> +       nbytes = cc_sg_trunc_len(sg, nbytes);
> +
>         if (sg_is_last(sg)) {

(1) this branch is taken, and not the else below,
(2) nothing acts upon detecting nbytes = 0.

With extra debug print:

    cc_map_sg: nbytes  = 0, first branch taken

>                 /* One entry only case -set to DLLI */
>                 if (dma_map_sg(dev, sg, 1, direction) != 1) {
> @@ -313,12 +335,14 @@ static int cc_map_sg(struct device *dev, struct scatterlist *sg,
>                 /* In case of mmu the number of mapped nents might
>                  * be changed from the original sgl nents
>                  */
> -               *mapped_nents = dma_map_sg(dev, sg, *nents, direction);
> -               if (*mapped_nents == 0) {
> +               ret = dma_map_sg(dev, sg, *nents, direction);
> +               if (dma_mapping_error(dev, ret)) {
>                         *nents = 0;
> -                       dev_err(dev, "dma_map_sg() sg buffer failed\n");
> +                       dev_err(dev, "dma_map_sg() sg buffer failed %d\n", ret);
>                         return -ENOMEM;
>                 }
> +
> +               *mapped_nents = ret;
>         }
>
>         return 0;

Gr{oetje,eeting}s,

                        Geert

-- 
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@...ux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds