linux-kernel - Re: [PATCH v2] serial: stm32: optimize spin lock usage

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: <YHlP/SryZXr/nNLM@hovoldconsulting.com>
Date:   Fri, 16 Apr 2021 10:51:09 +0200
From:   Johan Hovold <johan@...nel.org>
To:     Erwan LE RAY <erwan.leray@...s.st.com>
Cc:     dillon min <dillon.minfei@...il.com>,
        Greg KH <gregkh@...uxfoundation.org>, jirislaby@...nel.org,
        Maxime Coquelin <mcoquelin.stm32@...il.com>,
        Alexandre TORGUE <alexandre.torgue@...s.st.com>,
        kernel test robot <lkp@...el.com>,
        linux-serial@...r.kernel.org,
        linux-stm32@...md-mailman.stormreply.com,
        Linux ARM <linux-arm-kernel@...ts.infradead.org>,
        Linux Kernel Mailing List <linux-kernel@...r.kernel.org>,
        kbuild-all@...ts.01.org, clang-built-linux@...glegroups.com,
        Gerald Baeza <gerald.baeza@...s.st.com>
Subject: Re: [PATCH v2] serial: stm32: optimize spin lock usage

[ Please avoid top-posting. ]

On Thu, Apr 15, 2021 at 07:09:14PM +0200, Erwan LE RAY wrote:
> Hi Dillon,
> 
> STM32MP151 is mono-core, but both STM32MP153 and STM32MP157 are 
> dual-core (see 
> https://www.st.com/content/st_com/en/products/microcontrollers-microprocessors/stm32-arm-cortex-mpus.html).
> So your point is fully relevant, thanks.
> 
> ST already fixed the same issue in st-asc.c driver in the past (see 
> ef49ffd8), because a systematic deadlock was detected with RT kernel.

That's not the same issue. The above mentioned commit fixed an issue on
*RT* where local_irq_save() should be avoided.

> You proposed a first implementation in your patch, and a second one in 
> the discussion. It seems that your initial proposal (ie your V2 patch) 
> is the most standard one (implemented in 6 drivers). The second 
> implementation is implemented by only 1 company.
> 
> It looks that the solution is to avoid locking in the sysrq case and 
> trylock in the oops_in_progress case (see detailed analysis in 
> 677fe555cbfb1).
>
> So your initial patch looks to the right proposal, but it would be safer 
> if Greg could confirm it.

That would only fix the RT issue (and by making the sysrq one slightly
worse).

Using uart_unlock_and_check_sysrq() would address both issues.

Johan