On 20.02.24 23:23, Jarkko Sakkinen wrote: > On Tue Feb 20, 2024 at 8:54 PM UTC, Lino Sanfilippo wrote: >> Hi, >> >> On 20.02.24 19:42, Alexander Steffen wrote: >>> ATTENTION: This e-mail is from an external sender. Please check attachments and links before opening e.g. with mouseover. >>> >>> >>> On 02.02.2024 04:08, Lino Sanfilippo wrote: >>>> On 01.02.24 23:21, Jarkko Sakkinen wrote: >>>> >>>>> >>>>> On Wed Jan 31, 2024 at 7:08 PM EET, Daniel P. Smith wrote: >>>>>> Commit 933bfc5ad213 introduced the use of a locality counter to control when a >>>>>> locality request is allowed to be sent to the TPM. In the commit, the counter >>>>>> is indiscriminately decremented. Thus creating a situation for an integer >>>>>> underflow of the counter. >>>>> >>>>> What is the sequence of events that leads to this triggering the >>>>> underflow? This information should be represent in the commit message. >>>>> >>>> >>>> AFAIU this is: >>>> >>>> 1. We start with a locality_counter of 0 and then we call tpm_tis_request_locality() >>>> for the first time, but since a locality is (unexpectedly) already active >>>> check_locality() and consequently __tpm_tis_request_locality() return "true". >>> >>> check_locality() returns true, but __tpm_tis_request_locality() returns >>> the requested locality. Currently, this is always 0, so the check for >>> !ret will always correctly indicate success and increment the >>> locality_count. >>> >> >> Will the TPM TIS CORE ever (have to) request another locality than 0? Maybe the best would >> be to hardcode TPM_ACCESS(0) and get rid of all the locality parameters that are >> passed from one function to another. > > Usually, or at least use cases I'm aware of, localities are per > component. E.g. Intel TXT has one and Linux has another. > > There's been some proposals in the past here for hypervisor specific > locality here at LKML they didn't lead to anything. > > If you are suggesting of removing "int l" parameter altogether, I > do support that idea. > Yes, removing the "l" parameter is what I meant. I can prepare a patch for the removal. Regards, Lino >> But this is rather code optimization and not really required to fix >> the reported bug. > > Just adding here that I wish we also had a log transcript of bug, which > is right now missing. The explanation believable enough to move forward > but I still wish to see a log transcript. > > A/B testing of the bug and fix is something I'm lacking here. If anyone > has ideas how to use QEMU to simulate what Intel TXT does with > localities that would help. > > Most of us do not carry Intel TXT setup anywhere (home or office). > > Also even tho 0/3 has an explanation bug 1/3 does not have anything at > all to make it to be counted as a bug fix. Pretty difficult to compare > any possible proposals for a fix on this playground tbh. > > BR, Jarkko >
