Re: [PATCH RESEND] crypto: af_alg - add keylen checking to avoid NULL ptr passing down

[Eric Biggers <ebiggers3@gmail.com>]

On Mon, Dec 18, 2017 at 01:40:36PM +0000, Li Kun wrote:
> alg_setkey do not check the keylen whether it is zero, so the key
> may be ZERO_SIZE_PTR when keylen is 0, which will pass the
> copy_from_user's checking and be passed to the lower functions as key.
> 
> If the lower functions only check the key if it is NULL, ZERO_SIZE_PTR
> will pass the checking, and will cause null ptr dereference, so it's
> better to intercept the invalid parameters in the upper functions.
> 
> This patch is also suitable to fix CVE-2017-15116 for stable trees.
> 
> Signed-off-by: Li Kun <hw.likun@huawei.com>
> Cc: stable@vger.kernel.org
> ---
>  crypto/af_alg.c | 2 ++
>  1 file changed, 2 insertions(+)
> 
> diff --git a/crypto/af_alg.c b/crypto/af_alg.c
> index 337cf38..10f22f3 100644
> --- a/crypto/af_alg.c
> +++ b/crypto/af_alg.c
> @@ -210,6 +210,8 @@ static int alg_setkey(struct sock *sk, char __user *ukey,
>  	u8 *key;
>  	int err;
>  
> +	if (!keylen)
> +		return -EINVAL;
>  	key = sock_kmalloc(sk, keylen, GFP_KERNEL);
>  	if (!key)
>  		return -ENOMEM;
> -- 

If the length is 0 then why is the underlying ->setkey() method dereferencing
the pointer?  Which algorithm does this happen for?  Checking for NULL makes no
sense; the length needs to be checked instead.

Eric