Re: [PATCH v2 1/1] tpm: add sysfs exports for all banks of PCR registers

[Jerry Snitselaar <jsnitsel@redhat.com>]

James Bottomley @ 2020-07-21 16:37 MST:

> On Tue, 2020-07-21 at 16:16 -0700, Jerry Snitselaar wrote:
>> James Bottomley @ 2020-07-21 08:56 MST:
> [...]
>> > +	/*
>> > +	 * This will only trigger if someone has added an
>> > additional
>> > +	 * hash to the tpm_algorithms enum without incrementing
>> > +	 * TPM_MAX_HASHES.  This has to be a BUG_ON because under
>> > this
>> > +	 * condition, the chip->groups array will overflow
>> > corrupting
>> > +	 * the chips structure.
>> > +	 */
>> > +	BUG_ON(chip->groups_cnt > TPM_MAX_HASHES);
>> 
>> Should this check be 3 + TPM_MAX_HASHES like below?
>
> No, because at this point only a single additional group has been
> addedin addition to the hashes groups.  The first line of
> tpm_sysfs_add_device is
>
> 	WARN_ON(chip->groups_cnt != 0);
>
> And then we add the unnamed group.  This loop over the banks follows
> it, so chip->groups_cnt should be nr_banks_allocated by the end (it's
> the index, which is one fewer than the number of entries in chip-
>>groups[]).  We have a problem if nr_banks_allocated > TPM_MAX_HASHES
> which is what the BUG_ON checks.
>
> James

If the chip supported all 5 listed cases wouldn't groups_cnt be 6 at this
point?