Re: [PATCH] block: Add config option to not allow writing to mounted devices

["Colin Walters" <walters@verbum.org>]

On Mon, Jun 12, 2023, at 1:39 PM, Bart Van Assche wrote:
> On 6/12/23 09:25, Jan Kara wrote:
>> On Mon 12-06-23 18:16:14, Jan Kara wrote:
>>> Writing to mounted devices is dangerous and can lead to filesystem
>>> corruption as well as crashes. Furthermore syzbot comes with more and
>>> more involved examples how to corrupt block device under a mounted
>>> filesystem leading to kernel crashes and reports we can do nothing
>>> about. Add config option to disallow writing to mounted (exclusively
>>> open) block devices. Syzbot can use this option to avoid uninteresting
>>> crashes. Also users whose userspace setup does not need writing to
>>> mounted block devices can set this config option for hardening.
>>>
>>> Link: https://lore.kernel.org/all/60788e5d-5c7c-1142-e554-c21d709acfd9@linaro.org
>>> Signed-off-by: Jan Kara <jack@suse.cz>
>> 
>> Please disregard this patch. I had uncommited fixups in my tree. I'll send
>> fixed version shortly. I'm sorry for the noise.
>
> Have alternatives been configured to making this functionality configurable
> at build time only? How about a kernel command line parameter instead of a
> config option?

It's not just syzbot here; at least once in my life I accidentally did `dd if=/path/to/foo.iso of=/dev/sda` when `/dev/sda` was my booted disk and not the target USB device.  I know I'm not alone =)

There's a lot of similar accidental-damage protection from this.  Another stronger argument here is that if one has a security policy that restricts access to filesystem level objects, if a process can somehow write to a mounted block device, it effectively subverts all of those controls. 

Right now it looks to me we're invoking devcgroup_check_permission pretty early on; maybe we could extend the device cgroup stuff to have a new check for write-mounted, like

```
diff --git a/tools/include/uapi/linux/bpf.h b/tools/include/uapi/linux/bpf.h
index c994ff5b157c..f2af33c5acc1 100644
--- a/tools/include/uapi/linux/bpf.h
+++ b/tools/include/uapi/linux/bpf.h
@@ -6797,6 +6797,7 @@ enum {
 	BPF_DEVCG_ACC_MKNOD	= (1ULL << 0),
 	BPF_DEVCG_ACC_READ	= (1ULL << 1),
 	BPF_DEVCG_ACC_WRITE	= (1ULL << 2),
+	BPF_DEVCG_ACC_WRITE_MOUNTED	= (1ULL << 3),
 };
 
 enum {
```

?  But probably this would need to be some kind of opt-in flag to avoid breaking existing bpf progs?

If it was configurable via the device cgroup, then it's completely flexible from userspace; most specifically including supporting some specially privileged processes from doing it if necessary.

Also, I wonder if we should also support restricting *reads* from mounted block devices?