Re: [PATCH 1/2] zram: fix crashes due to use of cpu hotplug multistate

From: Kees Cook <keescook@chromium.org>
To: Greg KH <gregkh@linuxfoundation.org>
Cc: Jiri Kosina <jikos@kernel.org>,
	Thomas Gleixner <tglx@linutronix.de>,
	Luis Chamberlain <mcgrof@kernel.org>,
	Minchan Kim <minchan@kernel.org>,
	dhowells@redhat.com, hch@infradead.org, mbenes@suse.com,
	ngupta@vflare.org, sergey.senozhatsky.work@gmail.com,
	Jens Axboe <axboe@kernel.dk>,
	linux-block@vger.kernel.org, linux-kernel@vger.kernel.org
Subject: Re: [PATCH 1/2] zram: fix crashes due to use of cpu hotplug multistate
Date: Thu, 8 Apr 2021 20:01:11 -0700	[thread overview]
Message-ID: <202104081953.5A0D3923CE@keescook> (raw)
In-Reply-To: <YG7FGRNhIfDTqgUz@kroah.com>

On Thu, Apr 08, 2021 at 10:55:53AM +0200, Greg KH wrote:
> Module removal is not a "normal" operation that can be triggered by a
> system automatically without a user asking for it.  As someone reminded
> me on IRC, we used to do this "automatically" for many problematic
> drivers years ago for suspend/resume, that should all now be long fixed
> up.

I know what you're trying to say here, but I think you're just completely
wrong. :P

We absolutely must handle module unloading, and it is expected to work
correctly. _The reason it doesn't work correctly sometimes is because it
is a less common operation_. But that doesn't make it any less
important. Disk failures are rare too, but RAID handles it. If there are
bugs here it is due to _our lack of testing_.

Module unload needs to work for developers loading/unloading while they
work on a module[1], it needs to work for sysadmins that would to unload
a now-unused network protocol[2], it needs to work for people trying to
reset device hardware state[3], it needs to work for unloading unused
modules after root device, partition, and filesystem discovery in an
initramfs[3], and every other case.

The kernel module subsystem is not "best effort" and removal is not
"not normal". If unloading a module destabilizes the kernel, then there
is a bug that needs fixing. I'm not advocating for any path to fixing
the technical issues in this thread, but we absolutely cannot suddenly
claim that it's the user's fault that their system dies if they use
"rmmod". That's outrageous. O_o

-Kees

[1] Yes, I'm linking to the book you wrote ;)
    https://www.oreilly.com/library/view/linux-device-drivers/0596005903/ch02.html
[2] https://www.cs.unh.edu/cnrg/people/gherrin/linux-net.html#tth_sEc11.2.2
[3] https://opensource.com/article/18/5/how-load-or-unload-linux-kernel-module
    https://access.redhat.com/documentation/en-us/red_hat_enterprise_linux/6/html/deployment_guide/sec-unloading_a_module
[4] https://git.busybox.net/busybox/tree/modutils/rmmod.c

-- 
Kees Cook