CVE-2024-43898 is invalid?

CVE-2024-43898 is invalid?

[Theodore Ts'o]

I believe CVE-2024-43898 regarding "ext4: sanity check for NULL
pointer after ext4_force_shutdown" (commit id: 83f4414b8f84) may have
been issued in error.

ext4_force_shutdown() is called from FS_IOC_SHUTDOWN, which requires
root privileges.

Cheers,

						- Ted

Re: CVE-2024-43898 is invalid?

[Greg KH]

On Mon, Sep 09, 2024 at 11:31:44AM -0400, Theodore Ts'o wrote:
> I believe CVE-2024-43898 regarding "ext4: sanity check for NULL
> pointer after ext4_force_shutdown" (commit id: 83f4414b8f84) may have
> been issued in error.
> 
> ext4_force_shutdown() is called from FS_IOC_SHUTDOWN, which requires
> root privileges.

"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?

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.

thanks,

greg k-h

Re: CVE-2024-43898 is invalid?

[Theodore Ts'o]

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@kernel.org 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.

> 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.

Thanks,

						- Ted

Re: CVE-2024-43898 is invalid?

[Greg KH]

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@kernel.org 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