public inbox for linux-kernel@vger.kernel.org
 help / color / mirror / Atom feed
From: "Jarkko Sakkinen" <jarkko@kernel.org>
To: "Hao Ge" <hao.ge@linux.dev>, <peterhuewe@gmx.de>, <jgg@ziepe.ca>
Cc: <linux-integrity@vger.kernel.org>, <linux-kernel@vger.kernel.org>,
	"Hao Ge" <gehao@kylinos.cn>
Subject: Re: [PATCH] tpm: Move dereference after NULL check in tpm_buf_check_hmac_response
Date: Sun, 14 Jul 2024 18:43:44 +0300	[thread overview]
Message-ID: <D2PDLHX51C3K.16A4U6XFXRE29@kernel.org> (raw)
In-Reply-To: <20240709023337.102509-1-hao.ge@linux.dev>

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

  parent reply	other threads:[~2024-07-14 15:43 UTC|newest]

Thread overview: 13+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2024-07-09  2:33 [PATCH] tpm: Move dereference after NULL check in tpm_buf_check_hmac_response Hao Ge
2024-07-09  6:04 ` Markus Elfring
2024-07-14 15:43 ` Jarkko Sakkinen [this message]
2024-07-15  7:24   ` [PATCH v2] " Hao Ge
2024-07-15  8:29   ` [PATCH] " Hao Ge
2024-07-15 11:25 ` Jarkko Sakkinen
2024-07-15 11:52   ` James Bottomley
2024-07-16 10:06     ` Jarkko Sakkinen
2024-07-16  1:04   ` Hao Ge
2024-07-16 10:20     ` Jarkko Sakkinen
2024-07-16 10:33 ` Jarkko Sakkinen
2024-07-16 10:35   ` Jarkko Sakkinen
2024-07-16 10:57   ` Jarkko Sakkinen

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=D2PDLHX51C3K.16A4U6XFXRE29@kernel.org \
    --to=jarkko@kernel.org \
    --cc=gehao@kylinos.cn \
    --cc=hao.ge@linux.dev \
    --cc=jgg@ziepe.ca \
    --cc=linux-integrity@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=peterhuewe@gmx.de \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox