Messages in this thread | | | Date | Tue, 31 Jan 2017 22:50:06 +0200 | From | Jarkko Sakkinen <> | Subject | Re: Fwd: Re: [tpmdd-devel] [PATCH v9 2/2] tpm: add securityfs support,for TPM 2.0 firmware event log |
| |
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
| |