linux-kernel - Re: [patch] crypto: ccp - Fix SEV_VERSION_GREATER_OR

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: <e30eae0f-415b-842e-39c4-801227126367@amd.com>
Date:   Wed, 10 Jul 2019 22:06:49 +0000
From:   "Lendacky, Thomas" <Thomas.Lendacky@....com>
To:     David Rientjes <rientjes@...gle.com>,
        Herbert Xu <herbert@...dor.apana.org.au>
CC:     Cfir Cohen <cfir@...gle.com>,
        "Natarajan, Janakarajan" <Janakarajan.Natarajan@....com>,
        "Hook, Gary" <Gary.Hook@....com>,
        "linux-crypto@...r.kernel.org" <linux-crypto@...r.kernel.org>,
        "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>
Subject: Re: [patch] crypto: ccp - Fix SEV_VERSION_GREATER_OR_EQUAL

On 7/10/19 4:28 PM, David Rientjes wrote:
> SEV_VERSION_GREATER_OR_EQUAL() will fail if upgrading from 2.2 to 3.1, for
> example, because the minor version is not equal to or greater than the
> major.
> 
> Fix this and move to a static inline function for appropriate type
> checking.
> 
> Fixes: edd303ff0e9e ("crypto: ccp - Add DOWNLOAD_FIRMWARE SEV command")
> Reported-by: Cfir Cohen <cfir@...gle.com>
> Signed-off-by: David Rientjes <rientjes@...gle.com>
> ---
>  drivers/crypto/ccp/psp-dev.c | 19 ++++++++++++-------
>  1 file changed, 12 insertions(+), 7 deletions(-)
> 
> diff --git a/drivers/crypto/ccp/psp-dev.c b/drivers/crypto/ccp/psp-dev.c
> --- a/drivers/crypto/ccp/psp-dev.c
> +++ b/drivers/crypto/ccp/psp-dev.c
> @@ -24,10 +24,6 @@
>  #include "sp-dev.h"
>  #include "psp-dev.h"
>  
> -#define SEV_VERSION_GREATER_OR_EQUAL(_maj, _min)	\
> -		((psp_master->api_major) >= _maj &&	\
> -		 (psp_master->api_minor) >= _min)
> -
>  #define DEVICE_NAME		"sev"
>  #define SEV_FW_FILE		"amd/sev.fw"
>  #define SEV_FW_NAME_SIZE	64
> @@ -47,6 +43,15 @@ MODULE_PARM_DESC(psp_probe_timeout, " default timeout value, in seconds, during
>  static bool psp_dead;
>  static int psp_timeout;
>  
> +static inline bool sev_version_greater_or_equal(u8 maj, u8 min)
> +{
> +	if (psp_master->api_major > maj)
> +		return true;
> +	if (psp_master->api_major >= maj && psp_master->api_minor >= min)

Just a nit, but it reads a little clearer to me as:

	if (psp_master->api_major == maj && psp_master->api_minor >= min)

since you tested for major > above.

Thanks,
Tom

> +		return true;
> +	return false;
> +}
> +
>  static struct psp_device *psp_alloc_struct(struct sp_device *sp)
>  {
>  	struct device *dev = sp->dev;
> @@ -588,7 +593,7 @@ static int sev_ioctl_do_get_id2(struct sev_issue_cmd *argp)
>  	int ret;
>  
>  	/* SEV GET_ID is available from SEV API v0.16 and up */
> -	if (!SEV_VERSION_GREATER_OR_EQUAL(0, 16))
> +	if (!sev_version_greater_or_equal(0, 16))
>  		return -ENOTSUPP;
>  
>  	if (copy_from_user(&input, (void __user *)argp->data, sizeof(input)))
> @@ -651,7 +656,7 @@ static int sev_ioctl_do_get_id(struct sev_issue_cmd *argp)
>  	int ret;
>  
>  	/* SEV GET_ID available from SEV API v0.16 and up */
> -	if (!SEV_VERSION_GREATER_OR_EQUAL(0, 16))
> +	if (!sev_version_greater_or_equal(0, 16))
>  		return -ENOTSUPP;
>  
>  	/* SEV FW expects the buffer it fills with the ID to be
> @@ -1053,7 +1058,7 @@ void psp_pci_init(void)
>  		psp_master->sev_state = SEV_STATE_UNINIT;
>  	}
>  
> -	if (SEV_VERSION_GREATER_OR_EQUAL(0, 15) &&
> +	if (sev_version_greater_or_equal(0, 15) &&
>  	    sev_update_firmware(psp_master->dev) == 0)
>  		sev_get_api_version();
>  
>