netdev - Re: [PATCH V2] Fix a sleep-in-atomic bug in shash_setkey

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]

Date:   Mon, 2 Oct 2017 21:18:24 -0700
From:   Andy Lutomirski <luto@...nel.org>
To:     Jia-Ju Bai <baijiaju1990@....com>
Cc:     "David S. Miller" <davem@...emloft.net>,
        Herbert Xu <herbert@...dor.apana.org.au>,
        Neil Horman <nhorman@...driver.com>, vyasevich@...il.com,
        Andrew Lutomirski <luto@...nel.org>,
        Kalle Valo <kvalo@...eaurora.org>,
        Linux Crypto Mailing List <linux-crypto@...r.kernel.org>,
        Network Development <netdev@...r.kernel.org>,
        linux-sctp@...r.kernel.org,
        Linux Wireless List <linux-wireless@...r.kernel.org>
Subject: Re: [PATCH V2] Fix a sleep-in-atomic bug in shash_setkey_unaligned

> On Oct 2, 2017, at 7:25 PM, Jia-Ju Bai <baijiaju1990@....com> wrote:
>
> The SCTP program may sleep under a spinlock, and the function call path is:
> sctp_generate_t3_rtx_event (acquire the spinlock)
>  sctp_do_sm
>    sctp_side_effects
>      sctp_cmd_interpreter
>        sctp_make_init_ack
>          sctp_pack_cookie
>            crypto_shash_setkey
>              shash_setkey_unaligned
>                kmalloc(GFP_KERNEL)
>

I'm going to go out on a limb here: why on Earth is out crypto API so
full of indirection that we allocate memory at all here?

We're synchronously computing a hash of a small amount of data using
either HMAC-SHA1 or HMAC-SHA256 (determined at runtime) if I read it
right.  There's a sane way to do this that doesn't need kmalloc,
alloca, or fancy indirection.  And then there's crypto_shash_xyz().

--Andy, who is sick of seeing stupid bugs caused by the fact that it's
basically impossible to use the crypto API in a sane way.

P.S. gnulib has:

int hmac_sha256 (const void *key, size_t keylen, const void *in,
size_t inlen, void *resbuf);

An init/update/final-style API would be nice, but something like this
would be a phenomenal improvement over what we have.