Received-SPF: pass (google.com: best guess record for domain of linux-kernel-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) client-ip=209.132.180.67;
Date:   Mon, 5 Nov 2018 14:01:47 +0200
From:   Jarkko Sakkinen <jarkko.sakkinen@linux.intel.com>
To:     Roberto Sassu <roberto.sassu@huawei.com>
Cc:     Mimi Zohar <zohar@linux.ibm.com>, linux-integrity@vger.kernel.org,
        linux-security-module@vger.kernel.org,
        linux-kernel@vger.kernel.org, silviu.vlasceanu@huawei.com
Subject: Re: [PATCH v3 4/5] tpm: retrieve digest size of unknown algorithms
 with PCR read
Message-ID: <20181105120147.GA7621@linux.intel.com>
References: <20181030154711.2782-1-roberto.sassu@huawei.com>
 <20181030154711.2782-5-roberto.sassu@huawei.com>
 <1541088173.4035.26.camel@linux.ibm.com>
 <3034c896-788b-50f9-23cb-f4b2cd6363e6@huawei.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=iso-8859-1
Content-Disposition: inline
Content-Transfer-Encoding: 8bit
In-Reply-To: <3034c896-788b-50f9-23cb-f4b2cd6363e6@huawei.com>
Organization: Intel Finland Oy - BIC 0357606-4 - Westendinkatu 7, 02160 Espoo
User-Agent: Mutt/1.10.1 (2018-07-13)
Sender: linux-kernel-owner@vger.kernel.org
Precedence: bulk

On Mon, Nov 05, 2018 at 10:47:19AM +0100, Roberto Sassu wrote:
> > Commit?1db15344f874 ("tpm: implement TPM 2.0 capability to get active
> > PCR banks") defined active_banks[7]. ?Subsequently, commit
> > 4d23cc323cdb ("tpm: add securityfs support for TPM 2.0 firmware event
> > log") defined TPM2_PCR_ACTIVE_BANKS as 3. ?I'm not sure which is the
> > correct value, but the number of active_banks should not be hard coded
> > here.
> 
> Jarkko, should I change the value of TPM2_PCR_ACTIVE_BANKS, or set the
> size of the active_banks array to TPM2_PCR_ACTIVE_BANKS?

Hi, sorry I missed your patch set. Please add me either to 'To' or 'Cc'
field of the email if you want a quick response.

I think the implementation is flakky in both places and should be fixed
before doing any other changes. Thanks James for pointing out these
commits.

What you need to do is to create a prequel commit that reads the number
of banks to a variable e.g.

  unsigned int nr_active_banks;

and allocate 'active_banks' dynamically and change the places that
James pointed out. I guess it is OK to have a commit with two 'Fixes'
tags.

/Jarkko