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@xxxxxxxxxx >>> Signed-off-by: Jan Kara <jack@xxxxxxx> >> >> 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?
