linux-kernel - Re: [PATCH v4 1/6] tpm, tpm_tis

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: <Yn6esMyUl+QhECq+@kernel.org>
Date:   Fri, 13 May 2022 21:08:48 +0300
From:   Jarkko Sakkinen <jarkko@...nel.org>
To:     Lino Sanfilippo <LinoSanfilippo@....de>
Cc:     peterhuewe@....de, jgg@...pe.ca, stefanb@...ux.vnet.ibm.com,
        linux@...ewoehner.de, linux-integrity@...r.kernel.org,
        linux-kernel@...r.kernel.org, lukas@...ner.de,
        p.rosenberger@...bus.com, Lino Sanfilippo <l.sanfilippo@...bus.com>
Subject: Re: [PATCH v4 1/6] tpm, tpm_tis_spi: Request threaded irq

On Wed, May 11, 2022 at 09:18:39PM +0200, Lino Sanfilippo wrote:
> Hi,
> 
> On 11.05.22 at 13:22, Jarkko Sakkinen wrote:
> > On Mon, May 09, 2022 at 10:05:54AM +0200, Lino Sanfilippo wrote:
> >> From: Lino Sanfilippo <l.sanfilippo@...bus.com>
> >>
> >> Interrupt handling at least includes reading and writing the interrupt
> >> status register within the interrupt routine. Since accesses over the SPI
> >> bus are synchronized by a mutex, request a threaded interrupt handler to
> >> ensure a sleepable context during interrupt processing.
> >>
> >> Fixes: 1a339b658d9d ("tpm_tis_spi: Pass the SPI IRQ down to the driver")
> >> Signed-off-by: Lino Sanfilippo <l.sanfilippo@...bus.com>
> >
> > When you state that it needs a sleepable context, you should bring a
> > context why it needs it. This not to disregard the code change overally but
> > you cannot make even the most obvious claim without backing data.
> >
> 
> so what kind of backing data do you have in mind? Would it help to emphasize more
> that the irq handler is running in hard irq context in the current code and thus
> must not access registers over SPI since SPI uses a mutex (I consider it as basic
> knowledge that a mutex must not be taken in hard irq context)?

There's zero mention about specific lock you are talking about. Providing
the basic knowledge what you are trying to do is the whole point of the
commit message in the first place. I'd presume this patch is related to the
use bus_lock_mutex but it is fully ingored here.

BR, Jarkko