linux-kernel - Re: [PATCH] tpm: Move dereference after NULL check in tpm_buf_check_hmac

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-Id: <D2PDLHX51C3K.16A4U6XFXRE29@kernel.org>
Date: Sun, 14 Jul 2024 18:43:44 +0300
From: "Jarkko Sakkinen" <jarkko@...nel.org>
To: "Hao Ge" <hao.ge@...ux.dev>, <peterhuewe@....de>, <jgg@...pe.ca>
Cc: <linux-integrity@...r.kernel.org>, <linux-kernel@...r.kernel.org>, "Hao
 Ge" <gehao@...inos.cn>
Subject: Re: [PATCH] tpm: Move dereference after NULL check in
 tpm_buf_check_hmac_response

On Tue Jul 9, 2024 at 5:33 AM EEST, Hao Ge wrote:
> From: Hao Ge <gehao@...inos.cn>
>
> 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.

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.

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.

Address these issues and send v2. Thank you!

BR, Jarkko