tpm: Remove dead NULL check in tpm2_flush_space()

[PATCH] tpm: Remove dead NULL check in tpm2_flush_space()

Posted by Gunnar Kudrjavets 1 month, 2 weeks ago

The 'space' pointer in tpm2_flush_space() is assigned from
&chip->work_space, which is the address of an embedded struct member
within struct tpm_chip. This address can never be NULL, making the
NULL check dead code. The new code follows the existing pattern
established by the other callers in tpm2-space.c which also assign
from &chip->work_space without a NULL check. Remove the dead code
to avoid confusion.

Fixes: e3aaebcbb7c6 ("tpm: Clean up TPM space after command failure")
Signed-off-by: Gunnar Kudrjavets <gunnarku@amazon.com>
Assisted-by: Kiro:claude-opus-4.6
Reviewed-by: Justinien Bouron <jbouron@amazon.com>
---
 drivers/char/tpm/tpm2-space.c | 3 ---
 1 file changed, 3 deletions(-)

diff --git a/drivers/char/tpm/tpm2-space.c b/drivers/char/tpm/tpm2-space.c
index 60354cd53b5c..1eec72eb8208 100644
--- a/drivers/char/tpm/tpm2-space.c
+++ b/drivers/char/tpm/tpm2-space.c
@@ -169,9 +169,6 @@ void tpm2_flush_space(struct tpm_chip *chip)
 	struct tpm_space *space = &chip->work_space;
 	int i;
 
-	if (!space)
-		return;
-
 	for (i = 0; i < ARRAY_SIZE(space->context_tbl); i++)
 		if (space->context_tbl[i] && ~space->context_tbl[i])
 			tpm2_flush_context(chip, space->context_tbl[i]);

base-commit: 949692da7211572fac419b2986b6abc0cd1aeb76
-- 
2.47.3

Re: [PATCH] tpm: Remove dead NULL check in tpm2_flush_space()

Posted by Jarkko Sakkinen 1 month, 1 week ago

On Mon, Apr 27, 2026 at 04:32:26PM +0000, Gunnar Kudrjavets wrote:
> The 'space' pointer in tpm2_flush_space() is assigned from
> &chip->work_space, which is the address of an embedded struct member
> within struct tpm_chip. This address can never be NULL, making the
> NULL check dead code. The new code follows the existing pattern
> established by the other callers in tpm2-space.c which also assign
> from &chip->work_space without a NULL check. Remove the dead code
> to avoid confusion.
> 
> Fixes: e3aaebcbb7c6 ("tpm: Clean up TPM space after command failure")
> Signed-off-by: Gunnar Kudrjavets <gunnarku@amazon.com>
> Assisted-by: Kiro:claude-opus-4.6

Just for sake of understanding:

What is "kiro" and is assisted-by the tag supposed to be used here?

> Reviewed-by: Justinien Bouron <jbouron@amazon.com>
> ---
>  drivers/char/tpm/tpm2-space.c | 3 ---
>  1 file changed, 3 deletions(-)
> 
> diff --git a/drivers/char/tpm/tpm2-space.c b/drivers/char/tpm/tpm2-space.c
> index 60354cd53b5c..1eec72eb8208 100644
> --- a/drivers/char/tpm/tpm2-space.c
> +++ b/drivers/char/tpm/tpm2-space.c
> @@ -169,9 +169,6 @@ void tpm2_flush_space(struct tpm_chip *chip)
>  	struct tpm_space *space = &chip->work_space;
>  	int i;
>  
> -	if (!space)
> -		return;
> -
>  	for (i = 0; i < ARRAY_SIZE(space->context_tbl); i++)
>  		if (space->context_tbl[i] && ~space->context_tbl[i])
>  			tpm2_flush_context(chip, space->context_tbl[i]);
> 
> base-commit: 949692da7211572fac419b2986b6abc0cd1aeb76
> -- 
> 2.47.3
> 

It's all good otherwise, just need clarification as we are learning
how to deal with these patches :-)

BR, Jarkko

Re: [PATCH] tpm: Remove dead NULL check in tpm2_flush_space()

Posted by Gunnar Kudrjavets 1 month, 1 week ago

On Fri, May 09, 2026 at 11:28 AM Jarkko Sakkinen <jarkko@kernel.org> wrote:
> Just for sake of understanding:
>
> What is "kiro" and is assisted-by the tag supposed to be used here?

Kiro is an AI-powered IDE built on top of Claude [1]. In this case,
it helped with reasoning about the correctness of the fix, generating
test cases to validate the change, and formalizing the commit message.

I'm trying to follow the established guidelines for AI attribution
in kernel contributions to maintain scientific integrity [2] ;-)

On a more utopian note, it is likely that in a year or so there'll be
research conducted into how AI-assisted development is influencing the
changes to the kernel. My goal is to get into the habit of indicating
that AI was used to increase the number of valid data points.

[1] https://kiro.dev/
[2] https://docs.kernel.org/process/coding-assistants.html

Gunnar

Re: [PATCH] tpm: Remove dead NULL check in tpm2_flush_space()

Posted by Jarkko Sakkinen 1 month, 1 week ago

On Sat, May 09, 2026 at 10:54:49PM +0000, Gunnar Kudrjavets wrote:
> On Fri, May 09, 2026 at 11:28 AM Jarkko Sakkinen <jarkko@kernel.org> wrote:
> > Just for sake of understanding:
> >
> > What is "kiro" and is assisted-by the tag supposed to be used here?
> 
> Kiro is an AI-powered IDE built on top of Claude [1]. In this case,
> it helped with reasoning about the correctness of the fix, generating
> test cases to validate the change, and formalizing the commit message.
> 
> I'm trying to follow the established guidelines for AI attribution
> in kernel contributions to maintain scientific integrity [2] ;-)

OK, cool. I do know what Claude is :-) Just was unfamiliar term.

> 
> On a more utopian note, it is likely that in a year or so there'll be
> research conducted into how AI-assisted development is influencing the
> changes to the kernel. My goal is to get into the habit of indicating
> that AI was used to increase the number of valid data points.

Sure.

> 
> [1] https://kiro.dev/
> [2] https://docs.kernel.org/process/coding-assistants.html
> 
> Gunnar

BR, Jarkko

Re: [PATCH] tpm: Remove dead NULL check in tpm2_flush_space()

Posted by Jarkko Sakkinen 1 month, 1 week ago

On Sat, May 09, 2026 at 02:28:08PM +0300, Jarkko Sakkinen wrote:
> On Mon, Apr 27, 2026 at 04:32:26PM +0000, Gunnar Kudrjavets wrote:
> > The 'space' pointer in tpm2_flush_space() is assigned from
> > &chip->work_space, which is the address of an embedded struct member
> > within struct tpm_chip. This address can never be NULL, making the
> > NULL check dead code. The new code follows the existing pattern
> > established by the other callers in tpm2-space.c which also assign
> > from &chip->work_space without a NULL check. Remove the dead code
> > to avoid confusion.
> > 
> > Fixes: e3aaebcbb7c6 ("tpm: Clean up TPM space after command failure")
> > Signed-off-by: Gunnar Kudrjavets <gunnarku@amazon.com>
> > Assisted-by: Kiro:claude-opus-4.6
> 
> Just for sake of understanding:
> 
> What is "kiro" and is assisted-by the tag supposed to be used here?
> 
> > Reviewed-by: Justinien Bouron <jbouron@amazon.com>
> > ---
> >  drivers/char/tpm/tpm2-space.c | 3 ---
> >  1 file changed, 3 deletions(-)
> > 
> > diff --git a/drivers/char/tpm/tpm2-space.c b/drivers/char/tpm/tpm2-space.c
> > index 60354cd53b5c..1eec72eb8208 100644
> > --- a/drivers/char/tpm/tpm2-space.c
> > +++ b/drivers/char/tpm/tpm2-space.c
> > @@ -169,9 +169,6 @@ void tpm2_flush_space(struct tpm_chip *chip)
> >  	struct tpm_space *space = &chip->work_space;
> >  	int i;
> >  
> > -	if (!space)
> > -		return;
> > -
> >  	for (i = 0; i < ARRAY_SIZE(space->context_tbl); i++)
> >  		if (space->context_tbl[i] && ~space->context_tbl[i])
> >  			tpm2_flush_context(chip, space->context_tbl[i]);
> > 
> > base-commit: 949692da7211572fac419b2986b6abc0cd1aeb76
> > -- 
> > 2.47.3
> > 
> 
> It's all good otherwise, just need clarification as we are learning
> how to deal with these patches :-)

Hold on, I would not remove it: it's an invariant. It's not dead code.

BR, Jarkko

Re: [PATCH] tpm: Remove dead NULL check in tpm2_flush_space()

Posted by Paul Menzel 1 month, 2 weeks ago

Dear Gunnar,


Thank you for your patch.

Am 27.04.26 um 18:32 schrieb Gunnar Kudrjavets:
> The 'space' pointer in tpm2_flush_space() is assigned from
> &chip->work_space, which is the address of an embedded struct member
> within struct tpm_chip. This address can never be NULL, making the
> NULL check dead code. The new code follows the existing pattern
> established by the other callers in tpm2-space.c which also assign
> from &chip->work_space without a NULL check. Remove the dead code
> to avoid confusion.
> 
> Fixes: e3aaebcbb7c6 ("tpm: Clean up TPM space after command failure")
> Signed-off-by: Gunnar Kudrjavets <gunnarku@amazon.com>
> Assisted-by: Kiro:claude-opus-4.6
> Reviewed-by: Justinien Bouron <jbouron@amazon.com>
> ---
>   drivers/char/tpm/tpm2-space.c | 3 ---
>   1 file changed, 3 deletions(-)
> 
> diff --git a/drivers/char/tpm/tpm2-space.c b/drivers/char/tpm/tpm2-space.c
> index 60354cd53b5c..1eec72eb8208 100644
> --- a/drivers/char/tpm/tpm2-space.c
> +++ b/drivers/char/tpm/tpm2-space.c
> @@ -169,9 +169,6 @@ void tpm2_flush_space(struct tpm_chip *chip)
>   	struct tpm_space *space = &chip->work_space;
>   	int i;
>   
> -	if (!space)
> -		return;
> -
>   	for (i = 0; i < ARRAY_SIZE(space->context_tbl); i++)
>   		if (space->context_tbl[i] && ~space->context_tbl[i])
>   			tpm2_flush_context(chip, space->context_tbl[i]);

gemini/gemini-3.1-pro-preview made a comment [1]. No idea, if it’s valid.


Kind regards,

Paul


[1]: 
https://sashiko.dev/#/patchset/20260427163238.20230-1-gunnarku%40amazon.com

Re: [PATCH] tpm: Remove dead NULL check in tpm2_flush_space()

Posted by Gunnar Kudrjavets 1 month, 2 weeks ago

On Sun, Apr 27, 2026 at 10:49 PM Paul Menzel <pmenzel@molgen.mpg.de> wrote:
> gemini/gemini-3.1-pro-preview made a comment [1]. No idea, if it's valid.

Thanks for forwarding, Paul. AFAICS, the comment is a false positive.

My theory is that Gemini conflates two different variables named
'space':

1. The 'space' parameter passed to tpm_dev_transmit(). This *can* be
   NULL (it is NULL for /dev/tpm0 clients).

2. The local 'space' variable inside tpm2_flush_space(). This is
   assigned from &chip->work_space and can *never* be NULL.

The removed NULL check was testing case (2), not case (1).

Regards,
Gunnar

Re: [PATCH] tpm: Remove dead NULL check in tpm2_flush_space()

Posted by Paul Menzel 1 month, 2 weeks ago

Dear Gunnar,


Am 28.04.26 um 00:57 schrieb Gunnar Kudrjavets:
> On Sun, Apr 27, 2026 at 10:49 PM Paul Menzel <pmenzel@molgen.mpg.de> wrote:
>> gemini/gemini-3.1-pro-preview made a comment [1]. No idea, if it's valid.
> 
> Thanks for forwarding, Paul. AFAICS, the comment is a false positive.
> 
> My theory is that Gemini conflates two different variables named
> 'space':
> 
> 1. The 'space' parameter passed to tpm_dev_transmit(). This *can* be
>     NULL (it is NULL for /dev/tpm0 clients).
> 
> 2. The local 'space' variable inside tpm2_flush_space(). This is
>     assigned from &chip->work_space and can *never* be NULL.
> 
> The removed NULL check was testing case (2), not case (1).

Thank you for quickly looking into this. Sorry for the noise.


Kind regards,

Paul

Re: [PATCH] tpm: Remove dead NULL check in tpm2_flush_space()

Posted by Jarkko Sakkinen 1 month, 1 week ago

On Tue, Apr 28, 2026 at 08:18:09AM +0200, Paul Menzel wrote:
> Dear Gunnar,
> 
> 
> Am 28.04.26 um 00:57 schrieb Gunnar Kudrjavets:
> > On Sun, Apr 27, 2026 at 10:49 PM Paul Menzel <pmenzel@molgen.mpg.de> wrote:
> > > gemini/gemini-3.1-pro-preview made a comment [1]. No idea, if it's valid.
> > 
> > Thanks for forwarding, Paul. AFAICS, the comment is a false positive.
> > 
> > My theory is that Gemini conflates two different variables named
> > 'space':
> > 
> > 1. The 'space' parameter passed to tpm_dev_transmit(). This *can* be
> >     NULL (it is NULL for /dev/tpm0 clients).
> > 
> > 2. The local 'space' variable inside tpm2_flush_space(). This is
> >     assigned from &chip->work_space and can *never* be NULL.
> > 
> > The removed NULL check was testing case (2), not case (1).
> 
> Thank you for quickly looking into this. Sorry for the noise.

I felt sleep while reading the text generated by Gemini but yes exactly
because it can't be NULL the check is there.

I.e. Opus is right but the action taken is still plain wrong.

> Kind regards,
> 
> Paul

BR, Jarkko