Hi Jarkko
On 7/14/24 23:43, Jarkko Sakkinen wrote:
On Tue Jul 9, 2024 at 5:33 AM EEST, Hao Ge wrote:
From: Hao Ge <gehao@xxxxxxxxxx>
We shouldn't dereference "auth" until after we have checked that it is
non-NULL.
Sorry for some latency in responses. I'm on holiday up until week 31 and
only look at emerging issues but not every day.
Understand,I hope you enjoy your holiday.
I do agree with the code change but the description contains no information
of the bug and how the fix resolves it. Could you please rewrite the
description?
I can only think this realizing with tpm_ibmvtpm and TCG_TPM2_HMAC
enabled because it was according to recent learnings a platform which
does not end up calling tpm2_sessions_init().
Since you bug report contains no bug report, I need to ask that did you
realize a regression in some platform? Fix will get eventually accepted
even if the bug was found by "looking at code" but the gist here is that
your bug description contains no description how you found the bug,
which it should.
Actually It's reported by smatch and Dan Carpenter also noticed this
warning.
This is the email he sent
https://lore.kernel.org/all/3b1755a9-b12f-42fc-b26d-de2fe4e13ec2@stanley.mountain/
When TCG_TPM2_HMAC is disable it should be safe because we have:
static inline int tpm_buf_check_hmac_response(struct tpm_chip *chip,
struct tpm_buf *buf,
int rc)
{
return rc;
}
I also noticed that your fixes tag is incorrect as that null dereference
pre-existed on tpm_ibmvtpm before my fixes. It is not a new regression
introduced by my patches. So use git blame and check which commit
introduced that one.
I'm a bit confused about fixes tag. Sometimes tool often fail to truly
understand the context.
Form the logical perspective of the code,null deference indeed existed
before your fixes.
But for smatch, It simply conducted a static code analysis and flagged a
line with a warnning.
Its warning seems to be related to Commit 7ca110f2679b("tpm: Address
!chip->auth in tpm_buf_append_hmac_session*()")
That's why I used the 'fixes' tag for Commit 7ca110f2679b.(Please
forgive me for not being clear earlier. I discovered it through smatch.)
So, should I use the fix tag for Commit 7ca110f2679b ('tpm: Address
!chip->auth in tpm_buf_append_hmac_session*()') or for Commit
1085b8276bb4 ('tpm: Add the rest of the session HMAC API')?
Even though I have already send V2 which fixes tag using Commit
1085b8276bb4 ('tpm: Add the rest of the session HMAC API'),
I still want to gain knowledge in this area so that I can more
accurately use fxies tag and send patches in the future.
Address these issues and send v2. Thank you!
BR, Jarkko
Thanks
BR
Hao
