Received-SPF: pass (google.com: domain of linux-kernel+bounces-48960-linux.lists.archive=gmail.com@vger.kernel.org designates 2604:1380:45d1:ec00::1 as permitted sender) client-ip=2604:1380:45d1:ec00::1;
Precedence: bulk
Mime-Version: 1.0
Content-Transfer-Encoding: quoted-printable
Content-Type: text/plain; charset=UTF-8
Date: Fri, 02 Feb 2024 00:49:07 +0200
Message-Id: <CYU3XUGOX6QT.1GL070ONNPBWQ@suppilovahvero>
Cc: "Ross Philipson" <ross.philipson@oracle.com>, "Peter Huewe"
 <peterhuewe@gmx.de>
Subject: Re: [PATCH 3/3] tpm: make locality request return value consistent
From: "Jarkko Sakkinen" <jarkko@kernel.org>
To: "Daniel P. Smith" <dpsmith@apertussolutions.com>, "Jason Gunthorpe"
 <jgg@ziepe.ca>, <linux-integrity@vger.kernel.org>,
 <linux-kernel@vger.kernel.org>
References: <20240131170824.6183-1-dpsmith@apertussolutions.com>
 <20240131170824.6183-4-dpsmith@apertussolutions.com>
In-Reply-To: <20240131170824.6183-4-dpsmith@apertussolutions.com>

On Wed Jan 31, 2024 at 7:08 PM EET, Daniel P. Smith wrote:
> The function tpm_tis_request_locality() is expected to return the localit=
y
> value that was requested, or a negative error code upon failure. If it is=
 called
> while locality_count of struct tis_data is non-zero, no actual locality r=
equest
> will be sent. Because the ret variable is initially set to 0, the
> locality_count will still get increased, and the function will return 0. =
For a
> caller, this would indicate that locality 0 was successfully requested an=
d not
> the state changes just mentioned.
>
> Additionally, the function __tpm_tis_request_locality() provides inconsis=
tent
> error codes. It will provide either a failed IO write or a -1 should it h=
ave
> timed out waiting for locality request to succeed.
>
> This commit changes __tpm_tis_request_locality() to return valid negative=
 error
> codes to reflect the reason it fails. It then adjusts the return value ch=
eck in
> tpm_tis_request_locality() to check for a non-negative return value befor=
e
> incrementing locality_cout. In addition, the initial value of the ret val=
ue is
> set to a negative error to ensure the check does not pass if
> __tpm_tis_request_locality() is not called.

This is way way too abtract explanation and since I don't honestly
understand what I'm reading, the code changes look bunch of arbitrary
changes with no sound logic as a whole.

Please do not add more text to it. Instead write something more
understandable.

Again, we would need a legit scenario for each and any return value
change.<F49>

BR, Jarkko