linux-kernel - Re: [PATCH] fs: crypto: keyinfo: Fix a possible null-pointer dereference in derive_key

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: <20190724160711.GB673@sol.localdomain>
Date:   Wed, 24 Jul 2019 09:07:11 -0700
From:   Eric Biggers <ebiggers@...nel.org>
To:     Jia-Ju Bai <baijiaju1990@...il.com>
Cc:     tytso@....edu, jaegeuk@...nel.org, linux-fscrypt@...r.kernel.org,
        linux-kernel@...r.kernel.org, linux-crypto@...r.kernel.org
Subject: Re: [PATCH] fs: crypto: keyinfo: Fix a possible null-pointer
 dereference in derive_key_aes()

[+Cc linux-crypto]

On Wed, Jul 24, 2019 at 06:02:04PM +0800, Jia-Ju Bai wrote:
> In derive_key_aes(), tfm is assigned to NULL on line 46, and then
> crypto_free_skcipher(tfm) is executed.
> 
> crypto_free_skcipher(tfm)
>     crypto_skcipher_tfm(tfm)
>         return &tfm->base;
> 
> Thus, a possible null-pointer dereference may occur.

This analysis is incorrect because only the address &tfm->base is taken.
There's no pointer dereference.

In fact all the crypto_free_*() functions are no-ops on NULL pointers, and many
other callers rely on it.  So there's no bug here.

It appears you've sent the same patch for some of these other callers
(https://lore.kernel.org/lkml/?q=%22fix+a+possible+null-pointer%22), but none
are Cc'ed to linux-crypto or another mailing list I'm subscribed to, so I can't
respond to them.  But this feedback applies equally to them too.

Note also that if there actually were a bug here (which again, there doesn't
appear to be), we'd need to fix it in crypto_free_*(), not in the callers.

- Eric