On Tue, Jun 13, 2023 at 01:34:48PM +0200, Jan Kara wrote: > On Mon 12-06-23 14:52:54, Colin Walters wrote: > > 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 =) > > Yeah, so I'm not sure we are going to protect against this particular case. > I mean it is not *that* uncommon to alter partition table of /dev/sda while > /dev/sda1 is mounted. And for the kernel it is difficult to distinguish > this and your mishap. > > > 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. > > Well, there are multiple levels of protection that I can think of: > > 1) If user can write some image and make kernel mount it. > 2) If user can modify device content while mounted (but not buffer cache > of the device). > 3) If user can modify buffer cache of the device while mounted. > > 3) is the most problematic and effectively equivalent to full machine > control (executing arbitrary code in kernel mode) these days. For 1) and > 2) there are reasonable protection measures the filesystem driver can take > (and effectively you cannot escape these problems if you allow attaching > untrusted devices such as USB sticks) so they can cause DoS but we should > be able to prevent full machine takeover in the filesystem code. > > So this patch is mainly aimed at forbiding 3). > > > 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. > > I kind of like the flexibility of device cgroups but it does not seem to Let's not bring in device cgroups here just yet. They're an optional LSM security measure while your change is more fundamental which is the right thing to do imho.
