All of lore.kernel.org
 help / color / mirror / Atom feed
From: Vincent Whitchurch <vincent.whitchurch@axis.com>
To: Jarkko Sakkinen <jarkko@kernel.org>
Cc: Peter Huewe <peterhuewe@gmx.de>, Jason Gunthorpe <jgg@ziepe.ca>,
	kernel <kernel@axis.com>,
	"linux-integrity@vger.kernel.org"
	<linux-integrity@vger.kernel.org>,
	"linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
	<James.Bottomley@HansenPartnership.com>,
	<l.sanfilippo@kunbus.com>
Subject: Re: [PATCH 2/2] tpm: Fix crash on tmprm release
Date: Thu, 17 Jun 2021 07:44:58 +0200	[thread overview]
Message-ID: <20210617054458.GB4049@axis.com> (raw)
In-Reply-To: <20210615131848.cynblt5qindtvtiz@kernel.org>

On Tue, Jun 15, 2021 at 03:18:48PM +0200, Jarkko Sakkinen wrote:
> On Tue, Jun 15, 2021 at 11:14:09AM +0200, Vincent Whitchurch wrote:
> > If the tpm_tis module is removed (or a system shutdown is triggered)
> > while the tpmrm device is use, the kernel crashes due to chip->ops being
> > NULL:
> > 
> >  # exec 3<>/dev/tpmrm0
> >  # rmmod tpm_tis
> >  # exit
> >  ==================================================================
> >  BUG: KASAN: null-ptr-deref in tpm_chip_start+0x2d/0x120 [tpm]
> >  Read of size 8 at addr 0000000000000060 by task sh/994
> > 
> >  Call Trace:
> >   kasan_report.cold.13+0x10f/0x111
> >   tpm_chip_start+0x2d/0x120 [tpm]
> >   tpm2_del_space+0x2c/0xa0 [tpm]
> >   tpmrm_release+0x3f/0x50 [tpm]
> >   __fput+0x110/0x3c0
> >   task_work_run+0x94/0xd0
> >   do_exit+0x683/0x13e0
> >   do_group_exit+0x8b/0x140
> >   do_syscall_64+0x3c/0x80
> >  ==================================================================
> > 
> > Fix this by making tpm2_del_space() use tpm_try_get_ops().  The latter
> > already includes the calls to tpm_chip_start() and tpm_chip_stop().
> 
> This lacks explanation why migrating to tpm_try_get_ops() fixes the
> issue. Saying that doing something fixes something is not good enough
> explanation. So, can you extend this paragraph just a bit in the next
> version?

Thank you for the review, but I see now that James already proposed a
more-or-less identical fix earlier, so I'll let that patch run its due
course:

 https://lore.kernel.org/linux-integrity/e7566e1e48f5be9dca034b4bfb67683b5d3cb88f.camel@HansenPartnership.com/
 https://lore.kernel.org/linux-integrity/327f4c87-e652-6cbe-c624-16a6edf0c31d@kunbus.com/

  reply	other threads:[~2021-06-17  5:45 UTC|newest]

Thread overview: 7+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-06-15  9:14 [PATCH 1/2] tpm: Fix tpmrm reference counting Vincent Whitchurch
2021-06-15  9:14 ` [PATCH 2/2] tpm: Fix crash on tmprm release Vincent Whitchurch
2021-06-15 13:18   ` Jarkko Sakkinen
2021-06-17  5:44     ` Vincent Whitchurch [this message]
2022-03-03  2:03   ` Stefan Berger
2021-06-16 18:53 ` [PATCH 1/2] tpm: Fix tpmrm reference counting Jason Gunthorpe
2021-06-17  5:38   ` Vincent Whitchurch

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=20210617054458.GB4049@axis.com \
    --to=vincent.whitchurch@axis.com \
    --cc=James.Bottomley@HansenPartnership.com \
    --cc=jarkko@kernel.org \
    --cc=jgg@ziepe.ca \
    --cc=kernel@axis.com \
    --cc=l.sanfilippo@kunbus.com \
    --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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.