On Mon, Sep 09, 2024 at 04:08:22PM -0400, Theodore Ts'o wrote: > On Mon, Sep 09, 2024 at 06:20:08PM +0200, Greg KH wrote: > > > > "root privileges" are not something that "is this a vulnerability" > > normally takes into account given that there are zillions of ways of > > giving permissions to processes to do things that people do in crazy > > systems, as you know :) > > > > That being said, the commit message does not document root priviliges > > being needed, also, it looks like the function is called on the "normal" > > shutdown callback for the superblock, which I don't think is required to > > have root permissions, does it? > > It's fair that while umount(2) requires root privs, it's absolutely > true that there are a number of ways that an unprivileged user might > be able to request that the system unmount a file system on its > behalf. However, this particular failure involves a forcible shutdown > (triggered via ext4_force_shutdown() and the FS_IOC_SHUTDOWN ioctl) > without any regard to whether the file system is busy or not. A > "normal" superblock shutdown via umount(2) would never hit this > scenario because the umount(2) would return EBUSY if there are any > open file descriptors, and the syzkaller reproducer involves doing a > lot of file system operations racing with the FS_IOC_SHUTDOWN ioctl. > > The FS_IOC_SHUTDOWN ioctl is used for debugging and testing, and it's > not something that will be triggered by some setuid program or some > root daemon like udisks or udev. This is why I had intentionally > skipped adding a cc: stable@xxxxxxxxxx for this particular patch. > > It's fair to say that we didn't explicitly say that root was required; > we can try to be a bit more explicit about whether something is > legitimately a security fix or not. At least in my mind, it was so > obviously not that I didn't bother to say so, other than _not_ cc'ing > stable, which is not necessarily an obvious statement since it could > have been an oversight. I'll try to be more explicit in the future. You don't have to be more explicit, except maybe for things that "look" like they should be backported and are fixes needed but really should not be, like this one. > > But as a maintainer, it's up to you if you wish to reject a cve for your > > subsystem/code, so if you really want it rejected, we'll be glad to do > > so. > > There are some more borderline cases, such as people who enable their > systems to automount USB thumb drives which users find scattered in a > parking lot by a nation-state attacker. (Note: both xfsprogs and > e2fsprogs now ship with udev rules which disable this feature by > default; that won't stop a distro product manager for thinking that > user friendliness trumps security.) > > But in this case, this is so far outside the normal parameters that > yes, let's reject this particular CVE since it will be a vulnerability > for essentially no one. Now rejected, thanks! greg k-h
