Re: Fwd: Re: [tpmdd-devel] [PATCH v9 2/2] tpm: add securityfs support,for TPM 2.0 firmware event log

[Jarkko Sakkinen]

On Wed, Feb 01, 2017 at 12:14:12AM +0530, Nayna wrote:
> > I already sent my pull request to 4.11 and even today I found something
> > fishy. You declared a function local array by using a variable in "tpm:
> > enhance TPM 2.0 PCR extend to support multiple banks" (max_active_banks
> > or something). And the event log patches have just passed the review.
> 
> Yes. I have checked using clang and it has passed the clang.. and I also
> verified there were no complains during build.

What we can deduce from that is that they didn't expose the issue in
question.

I found this by running sparse with make C=2 M=drives/char/tpm

> What type of problem do you see ?

It is disallowed to do stack allocation in the kernel code even if C
standard would allow it. Stack is scarce resource so you need to know
its usage at compile time.

In this case you actually know the allocation because the value is not
changed during the course of the function but it is still bad. Probably
compiler will optimize it out. Still it is not a good practice.

> Also, to understand, this is related to multi-bank patchset. I mean how does
> it affect for event log patchset ?

Well in both cases these have landed fairly late but I asked from James
whether I'll have to postpone these to 4.12.

Usually when I've sent my release pull request I do not want to make any
radical changes to the codebase because they always require extra QA and
thus take extra time.

/Jarkko