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:   Tue, 3 Jan 2023 08:39:45 -0600
From:   Tom Lendacky <thomas.lendacky@....com>
To:     David Rientjes <rientjes@...gle.com>,
        Herbert Xu <herbert@...dor.apana.org.au>,
        "David S. Miller" <davem@...emloft.net>
Cc:     Peter Gonda <pgonda@...gle.com>, Andy Nguyen <theflow@...gle.com>,
        linux-kernel@...r.kernel.org, linux-crypto@...r.kernel.org,
        John Allen <john.allen@....com>
Subject: Re: [patch] crypto: ccp - Avoid page allocation failure warning for
 SEV_GET_ID2

On 12/30/22 16:18, David Rientjes wrote:
> For SEV_GET_ID2, the user provided length does not have a specified
> limitation because the length of the ID may change in the future.  The
> kernel memory allocation, however, is implicitly limited to 4MB on x86 by
> the page allocator, otherwise the kzalloc() will fail.
> 
> When this happens, it is best not to spam the kernel log with the warning.
> Simply fail the allocation and return ENOMEM to the user.
> 
> Fixes: d6112ea0cb34 ("crypto: ccp - introduce SEV_GET_ID2 command")
> Reported-by: Andy Nguyen <theflow@...gle.com>
> Reported-by: Peter Gonda <pgonda@...gle.com>
> Suggested-by: Herbert Xu <herbert@...dor.apana.org.au>
> Signed-off-by: David Rientjes <rientjes@...gle.com>
> ---
>   drivers/crypto/ccp/sev-dev.c | 9 ++++++++-
>   1 file changed, 8 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/crypto/ccp/sev-dev.c b/drivers/crypto/ccp/sev-dev.c
> --- a/drivers/crypto/ccp/sev-dev.c
> +++ b/drivers/crypto/ccp/sev-dev.c
> @@ -881,7 +881,14 @@ static int sev_ioctl_do_get_id2(struct sev_issue_cmd *argp)
>   	input_address = (void __user *)input.address;
>   
>   	if (input.address && input.length) {
> -		id_blob = kzalloc(input.length, GFP_KERNEL);
> +		/*
> +		 * The length of the ID shouldn't be assumed by software since
> +		 * it may change in the future.  The allocation size is limited
> +		 * to 1 << (PAGE_SHIFT + MAX_ORDER - 1) by the page allocator.
> +		 * If the allocation fails, simply return ENOMEM rather than
> +		 * warning in the kernel log.
> +		 */
> +		id_blob = kzalloc(input.length, GFP_KERNEL | __GFP_NOWARN);

We could do this or we could have the driver invoke the API with a zero 
length to get the minimum buffer size needed for the call. The driver 
could then perform some validation checks comparing the supplied 
input.length to the returned length. If the driver can proceed, then if 
input.length is exactly 2x the minimum length, then kzalloc the 2 * 
minimum length, otherwise kzalloc the minimum length. This is a bit more 
complicated, though, compared to this fix.

Either way, is fine with me. Thoughts?

Thanks,
Tom

>   		if (!id_blob)
>   			return -ENOMEM;
>   

Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ