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]
Date:   Fri, 5 Oct 2018 10:27:48 +0200
From:   Nicolas Cavallari <Nicolas.Cavallari@...en-communications.fr>
To:     Russell King - ARM Linux <linux@...linux.org.uk>
Cc:     Andrew Morton <akpm@...ux-foundation.org>,
        linux-arm-kernel@...ts.infradead.org, linux-kernel@...r.kernel.org
Subject: Re: [RFC 1/2] reboot: Make restart_handler_list a blocking notifier
 chain.

On 04/10/2018 18:49, Russell King - ARM Linux wrote:
> This isn't going to work.
> 
> For example, sysrq processing (which can happen in IRQ context) calls
> emergency_restart() for the reboot sysrq.  That calls through to
> machine_restart(), which then calls do_kernel_restart().
>
> If do_kernel_restart() sleeps, then we're trying to sleep in IRQ
> context, and that's a no no.  I'm afraid you can't just add an irq
> enable and change the notifier list to be atomic - and, as you're
> making the change in generic code, this affects everyone, not just the
> architecture that happens to be in front of you (so if it were merged,
> you're likely to get a lot of bug reports!)

I don't get it.

Many implementations of machine_restart() sleeps or do an infinite loop.
Almost half of the restart_handler users sleeps or do an infinite loop.

I understand that sleeping in IRQ context is bad, but the kernel does it anyway.
And it seems nobody have noticed any problem with this.

The rn5t618 driver already does two I2C transfers in its restart handler.
Haven't anyone reported any problem about it ?

> It gets worse, because (eg) a panic() or IRQ can happen with any locks
> held - and if the I2C device's locks are held when one of those events
> happen, you have a deadlock situation (hence you won't reboot!)
> 
> I suppose a good first step would be for us to have our own
> machine_emergency_restart() on ARM, to separate the atomic paths
> from the non-atomic paths, so that those who need to talk to an I2C,
> that can happen from the non-atomic path (which means things like
> /sbin/reboot will work) but other stuff (eg, restart on panic, sysrq,
> soft-watchdog) will fail.

That would fix my use case, but not the existing code ?

Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ