linux-kernel - Re: [PATCH 1/1] trusted-keys: match tpm_get

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:   Thu, 29 Apr 2021 15:03:18 -0400
From:   Ben Boeckel <me@...boeckel.net>
To:     James Bottomley <James.Bottomley@...senpartnership.com>
Cc:     keyrings@...r.kernel.org, Ben Boeckel <mathstuf@...il.com>,
        linux-integrity@...r.kernel.org, linux-kernel@...r.kernel.org,
        linux-security-module@...r.kernel.org,
        Dan Carpenter <dan.carpenter@...cle.com>
Subject: Re: [PATCH 1/1] trusted-keys: match tpm_get_ops on all return paths

On Thu, Apr 29, 2021 at 11:50:50 -0700, James Bottomley wrote:
> Actually, I think this is a better fix to avoid multiple put and
> returns.
> 
> James
> 
> ---
> 
> diff --git a/security/keys/trusted-keys/trusted_tpm2.c b/security/keys/trusted-keys/trusted_tpm2.c
> index d225ad140960..cbf2a932577b 100644
> --- a/security/keys/trusted-keys/trusted_tpm2.c
> +++ b/security/keys/trusted-keys/trusted_tpm2.c
> @@ -336,9 +336,9 @@ int tpm2_seal_trusted(struct tpm_chip *chip,
>  			rc = -EPERM;
>  	}
>  	if (blob_len < 0)
> -		return blob_len;
> -
> -	payload->blob_len = blob_len;
> +		rc = blob_len;
> +	else
> +		payload->blob_len = blob_len;
>  
>  	tpm_put_ops(chip);
>  	return rc;

Ah, that does look better. I had first added a new label, but that
didn't seem like an improvement in readability. I grabbed this pattern
from an early return earlier in the function. But given that this is the
end (and appears to be unlikely to have more logic inserted in the
future), this seems more reasonable to me as well. Do you want me to
respin or just let it up to you at this point?

Thanks,

--Ben