Messages in this thread |  | | Subject | Re: Fwd: Re: [tpmdd-devel] [PATCH v9 2/2] tpm: add securityfs support,for TPM 2.0 firmware event log | From | Nayna <> | Date | Wed, 1 Feb 2017 13:01:06 +0530 |
| |
On 02/01/2017 02:20 AM, Jarkko Sakkinen wrote: > 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.
Thanks Jarkko for explaining it.
Hmm, do you want me to send a patch for this ? I think what we want is actually define it just array of size as 7.
> >> 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.
Thanks again for explaining details.
Thanks & Regards, - Nayna
> > /Jarkko >
|  |