Re: [PATCH] dm-verity: restart or panic on an I/O error

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Mon, Sep 30, 2024 at 10:10 AM Will Drewry <wad@xxxxxxxxxxxx> wrote:
>
> On Mon, Sep 30, 2024 at 11:27 AM Mikulas Patocka <mpatocka@xxxxxxxxxx> wrote:
> >
> >
> >
> > On Mon, 30 Sep 2024, Will Drewry wrote:
> >
> > > > The dm-verity behavior was reported as a security bug, so by default, it
> > > > should behave in the secure way - i.e. restart or panic on I/O error.
> > > >
> > > > Do you intend to use dm-verity in Android and ChromeOS in the less-secure
> > > > way where it returns -EIO? Have you audited the Android and ChromeOS
> > > > codebase so that -EIO can't cause security breach? If yes, I can make a
> > > > configuration switch for you that will enable the old behavior.
> > >
> > > tl;dr don't change the default behavior, but adding a reboot-on-eio is nice.
> >
> > OK, so I can revert it if you want it.
>
> That or an argument so the old behavior can remain for those who are using it
> (I can send a patch for that if it's easier too).

Thanks for chiming in on this, Will! I still think that adding a
separate restart_on_eio (or similar) flag would be the best solution.

> > I'd like to ask - there is another change in that patch - I changed
> >         kernel_restart("dm-verity device corrupted");
> > to
> >         pr_emerg("dm-verity device corrupted\n");
> >         emergency_restart();
> >
> > Because kernel_restart calls reboot notifiers and they may in theory wait
> > for the bio that caused the restart, resulting in deadlock.
> >
> > Do you want to have this part of the patch reverted too?
>
> IMHO, that's a good change!  If the policy is to restart on corruption, then
> it makes sense to avoid the reboot notifiers.

While I agree that this sounds good in principle, devices that use the
restart feature typically need to pass the reboot reason to a PMU, for
example, and it looks like Android devices depend on reboot notifiers
for this. Here's a recent Pixel implementation that checks for the
"dm-verity device corrupted" command as an example:

https://android.googlesource.com/kernel/google-modules/power/reset/+/refs/heads/android-gs-comet-6.1-android15/pixel-zuma-reboot.c#81

Sami





[Index of Archives]     [DM Crypt]     [Fedora Desktop]     [ATA RAID]     [Fedora Marketing]     [Fedora Packaging]     [Fedora SELinux]     [Yosemite Discussion]     [KDE Users]     [Fedora Docs]

  Powered by Linux