Re: CVE-2023-52596: sysctl: Fix out of bounds access for empty sysctl registers

Re: CVE-2023-52596: sysctl: Fix out of bounds access for empty sysctl registers

[Michal Hocko]

On Wed 06-03-24 06:45:54, Greg KH wrote:
> Description
> ===========
> 
> In the Linux kernel, the following vulnerability has been resolved:
> 
> sysctl: Fix out of bounds access for empty sysctl registers
> 
> When registering tables to the sysctl subsystem there is a check to see
> if header is a permanently empty directory (used for mounts). This check
> evaluates the first element of the ctl_table. This results in an out of
> bounds evaluation when registering empty directories.
> 
> The function register_sysctl_mount_point now passes a ctl_table of size
> 1 instead of size 0. It now relies solely on the type to identify
> a permanently empty register.
> 
> Make sure that the ctl_table has at least one element before testing for
> permanent emptiness.

While this makes the code more robust and more future proof I do not think
this is fixing any real issue not to mention anything with security
implications. AFAIU there is no actual code that can generate empty
sysctl directories unless the kernel is heavily misconfigured.

Luis, Joel, what do you think?

-- 
Michal Hocko
SUSE Labs

Re: CVE-2023-52596: sysctl: Fix out of bounds access for empty sysctl registers

[Luis Chamberlain]

On Mon, Mar 11, 2024 at 09:11:27AM +0100, Michal Hocko wrote:
> On Wed 06-03-24 06:45:54, Greg KH wrote:
> > Description
> > ===========
> > 
> > In the Linux kernel, the following vulnerability has been resolved:
> > 
> > sysctl: Fix out of bounds access for empty sysctl registers
> > 
> > When registering tables to the sysctl subsystem there is a check to see
> > if header is a permanently empty directory (used for mounts). This check
> > evaluates the first element of the ctl_table. This results in an out of
> > bounds evaluation when registering empty directories.
> > 
> > The function register_sysctl_mount_point now passes a ctl_table of size
> > 1 instead of size 0. It now relies solely on the type to identify
> > a permanently empty register.
> > 
> > Make sure that the ctl_table has at least one element before testing for
> > permanent emptiness.
> 
> While this makes the code more robust and more future proof I do not think
> this is fixing any real issue not to mention anything with security
> implications. AFAIU there is no actual code that can generate empty
> sysctl directories unless the kernel is heavily misconfigured.
> 
> Luis, Joel, what do you think?

As per review with Joel:

The out-of-bounds issue cannot be triggered in older kernels unless
you had external modules with empty sysctls. That is because although
support for empty sysctls was added on v6.6 none of the sysctls that
were trimmed for the superfluous entry over the different kernel
releases so far has had the chance to be empty.

The 0-day reported crash was for a future out of tree patch which was
never merged yet:

https://github.com/Joelgranados/linux/commit/423f75083acd947804c8d5c31ad1e1b5fcb3c020                                                                                                       

Let's examine this commit to see why it triggers a crash to understand
how the out of bounds issue can be triggered.

Looking for suspects of sysctls which may end up empty in that patch we
have a couple. kernel/sched/fair.c sched_fair_sysctls can end up empty when
you don't have CONFIG_CFS_BANDWIDTH or CONFIG_NUMA_BALANCING. But the kernel
config for the 0-day test was:                                                                                                                                                                         
https://download.01.org/0day-ci/archive/20231120/202311201431.57aae8f3-oliver.sang@intel.com/config-6.6.0-rc2-00030-g423f75083acd                                                           

It has CONFIG_CFS_BANDWIDTH=y but CONFIG_NUMA_BALANCING is not set so
this was not the culprit, but that configuration could have been a
cause if it was possible.

In the same future-not-upstream commit kernel/sched/core.c sched_core_sysctls
can be empty if you don't have the following:                                                                                                                     

CONFIG_SCHEDSTATS       --> not set on 0-day kernel
CONFIG_UCLAMP_TASK      --> not set on 0-day kernel                                                                                                                                         
CONFIG_NUMA_BALANCING   --> not set on 0-day kernel

So that was the cause, and an example real valid config which can trigger
a crash. But that patch was never upstream.

Now, we can look for existing removal of sysctl sentinels with:

git log -p --grep "superfluous sentinel element"

And of these, at first glance, only locks_sysctls seems like it *could*
possibly end up in a situation where random config would create an empty
sysctl, but exanding on the code we see that's not possible because
leases-enable sysctl entry is always built if you have sysctls enabled:

static struct ctl_table llocks_sysctlsocks_sysctls[] = {
        {
                .procname       = "leases-enable",
                .data           = &leases_enable,
                .maxlen         = sizeof(int),                                   
                .mode           = 0644,
                .proc_handler   = proc_dointvec,
        },
#ifdef CONFIG_MMU
        {
                .procname       = "lease-break-time",
                .data           = &lease_break_time,                             
                .maxlen         = sizeof(int),
                .mode           = 0644,
                .proc_handler   = proc_dointvec,
        },
#endif /* CONFIG_MMU */
};

The out of bounds fix commit should have just had the tag:

Fixes 9edbfe92a0a13 ("sysctl: Add size to register_sysctl")

Backporting this is fine, but wouldn't fix an issue unless an external
module had empty sysctls. And exploiting this is not possible unless
you purposely build an external module which could end up with empty
sysctls.

HTH,

  Luis

Re: CVE-2023-52596: sysctl: Fix out of bounds access for empty sysctl registers

[Lee Jones]

On Mon, 11 Mar 2024, Luis Chamberlain wrote:

> On Mon, Mar 11, 2024 at 09:11:27AM +0100, Michal Hocko wrote:
> > On Wed 06-03-24 06:45:54, Greg KH wrote:
> > > Description
> > > ===========
> > > 
> > > In the Linux kernel, the following vulnerability has been resolved:
> > > 
> > > sysctl: Fix out of bounds access for empty sysctl registers
> > > 
> > > When registering tables to the sysctl subsystem there is a check to see
> > > if header is a permanently empty directory (used for mounts). This check
> > > evaluates the first element of the ctl_table. This results in an out of
> > > bounds evaluation when registering empty directories.
> > > 
> > > The function register_sysctl_mount_point now passes a ctl_table of size
> > > 1 instead of size 0. It now relies solely on the type to identify
> > > a permanently empty register.
> > > 
> > > Make sure that the ctl_table has at least one element before testing for
> > > permanent emptiness.
> > 
> > While this makes the code more robust and more future proof I do not think
> > this is fixing any real issue not to mention anything with security
> > implications. AFAIU there is no actual code that can generate empty
> > sysctl directories unless the kernel is heavily misconfigured.
> > 
> > Luis, Joel, what do you think?
> 
> As per review with Joel:
> 
> The out-of-bounds issue cannot be triggered in older kernels unless
> you had external modules with empty sysctls. That is because although
> support for empty sysctls was added on v6.6 none of the sysctls that
> were trimmed for the superfluous entry over the different kernel
> releases so far has had the chance to be empty.
> 
> The 0-day reported crash was for a future out of tree patch which was
> never merged yet:
> 
> https://github.com/Joelgranados/linux/commit/423f75083acd947804c8d5c31ad1e1b5fcb3c020                                                                                                       
> 
> Let's examine this commit to see why it triggers a crash to understand
> how the out of bounds issue can be triggered.
> 
> Looking for suspects of sysctls which may end up empty in that patch we
> have a couple. kernel/sched/fair.c sched_fair_sysctls can end up empty when
> you don't have CONFIG_CFS_BANDWIDTH or CONFIG_NUMA_BALANCING. But the kernel
> config for the 0-day test was:                                                                                                                                                                         
> https://download.01.org/0day-ci/archive/20231120/202311201431.57aae8f3-oliver.sang@intel.com/config-6.6.0-rc2-00030-g423f75083acd                                                           
> 
> It has CONFIG_CFS_BANDWIDTH=y but CONFIG_NUMA_BALANCING is not set so
> this was not the culprit, but that configuration could have been a
> cause if it was possible.
> 
> In the same future-not-upstream commit kernel/sched/core.c sched_core_sysctls
> can be empty if you don't have the following:                                                                                                                     
> 
> CONFIG_SCHEDSTATS       --> not set on 0-day kernel
> CONFIG_UCLAMP_TASK      --> not set on 0-day kernel                                                                                                                                         
> CONFIG_NUMA_BALANCING   --> not set on 0-day kernel
> 
> So that was the cause, and an example real valid config which can trigger
> a crash. But that patch was never upstream.
> 
> Now, we can look for existing removal of sysctl sentinels with:
> 
> git log -p --grep "superfluous sentinel element"
> 
> And of these, at first glance, only locks_sysctls seems like it *could*
> possibly end up in a situation where random config would create an empty
> sysctl, but exanding on the code we see that's not possible because
> leases-enable sysctl entry is always built if you have sysctls enabled:
> 
> static struct ctl_table llocks_sysctlsocks_sysctls[] = {
>         {
>                 .procname       = "leases-enable",
>                 .data           = &leases_enable,
>                 .maxlen         = sizeof(int),                                   
>                 .mode           = 0644,
>                 .proc_handler   = proc_dointvec,
>         },
> #ifdef CONFIG_MMU
>         {
>                 .procname       = "lease-break-time",
>                 .data           = &lease_break_time,                             
>                 .maxlen         = sizeof(int),
>                 .mode           = 0644,
>                 .proc_handler   = proc_dointvec,
>         },
> #endif /* CONFIG_MMU */
> };
> 
> The out of bounds fix commit should have just had the tag:
> 
> Fixes 9edbfe92a0a13 ("sysctl: Add size to register_sysctl")
> 
> Backporting this is fine, but wouldn't fix an issue unless an external
> module had empty sysctls. And exploiting this is not possible unless
> you purposely build an external module which could end up with empty
> sysctls.

Thanks for the amazing explanation Luis.

If I'm reading this correctly, an issue does exist, but an attacker
would have to lay some foundations before it could be triggered.  Sounds
like loading of a malicious or naive module would be enough.

We know from conducting postmortems on previous exploits that successful
attacks often consist of leveraging a chain of smaller, seemingly
implausible or innocuous looking bugs rather than in isolation.

With that in mind, it is still my belief that this could be used by an
attacker in such a chain.  Unless I have this totally wrong or any of
the maintainers have strong feelings to the contrary, I would like to
keep the CVE number associated with this fix.

-- 
Lee Jones [李琼斯]

Re: CVE-2023-52596: sysctl: Fix out of bounds access for empty sysctl registers

[Michal Hocko]

On Tue 12-03-24 09:17:30, Lee Jones wrote:
[...]
> > Backporting this is fine, but wouldn't fix an issue unless an external
> > module had empty sysctls. And exploiting this is not possible unless
> > you purposely build an external module which could end up with empty
> > sysctls.

Thanks for the clarification Luis!

> Thanks for the amazing explanation Luis.
> 
> If I'm reading this correctly, an issue does exist, but an attacker
> would have to lay some foundations before it could be triggered.  Sounds
> like loading of a malicious or naive module would be enough.

If the bar is set as high as a kernel module to create and empty sysctl
directory then I think it is safe to say that the security aspect is
mostly moot. There are much simpler ways to attack the system if you are
able to load a kernel module.

> We know from conducting postmortems on previous exploits that successful
> attacks often consist of leveraging a chain of smaller, seemingly
> implausible or innocuous looking bugs rather than in isolation.
> 
> With that in mind, it is still my belief that this could be used by an
> attacker in such a chain.  Unless I have this totally wrong or any of
> the maintainers have strong feelings to the contrary, I would like to
> keep the CVE number associated with this fix.

No, no real strong feelings but I have to say that I find a CVE more
than a stretch. Kernel modules could do much more harm than just abuse
this particular bug.
-- 
Michal Hocko
SUSE Labs

Re: CVE-2023-52596: sysctl: Fix out of bounds access for empty sysctl registers

[Joel Granados]

[-- Attachment #1: Type: text/plain, Size: 5433 bytes --]

On Mon, Mar 11, 2024 at 02:57:50PM -0700, Luis Chamberlain wrote:
> On Mon, Mar 11, 2024 at 09:11:27AM +0100, Michal Hocko wrote:
> > On Wed 06-03-24 06:45:54, Greg KH wrote:
> > > Description
> > > ===========
> > > 
> > > In the Linux kernel, the following vulnerability has been resolved:
> > > 
> > > sysctl: Fix out of bounds access for empty sysctl registers
> > > 
> > > When registering tables to the sysctl subsystem there is a check to see
> > > if header is a permanently empty directory (used for mounts). This check
> > > evaluates the first element of the ctl_table. This results in an out of
> > > bounds evaluation when registering empty directories.
> > > 
> > > The function register_sysctl_mount_point now passes a ctl_table of size
> > > 1 instead of size 0. It now relies solely on the type to identify
> > > a permanently empty register.
> > > 
> > > Make sure that the ctl_table has at least one element before testing for
> > > permanent emptiness.
> > 
> > While this makes the code more robust and more future proof I do not think
> > this is fixing any real issue not to mention anything with security
> > implications. AFAIU there is no actual code that can generate empty
> > sysctl directories unless the kernel is heavily misconfigured.
> > 
> > Luis, Joel, what do you think?
> 
> As per review with Joel:
> 
> The out-of-bounds issue cannot be triggered in older kernels unless
> you had external modules with empty sysctls. That is because although
> support for empty sysctls was added on v6.6 none of the sysctls that
> were trimmed for the superfluous entry over the different kernel
> releases so far has had the chance to be empty.
> 
> The 0-day reported crash was for a future out of tree patch which was
> never merged yet:
> 
> https://github.com/Joelgranados/linux/commit/423f75083acd947804c8d5c31ad1e1b5fcb3c020                                                                                                       
> 
> Let's examine this commit to see why it triggers a crash to understand
> how the out of bounds issue can be triggered.
> 
> Looking for suspects of sysctls which may end up empty in that patch we
> have a couple. kernel/sched/fair.c sched_fair_sysctls can end up empty when
> you don't have CONFIG_CFS_BANDWIDTH or CONFIG_NUMA_BALANCING. But the kernel
> config for the 0-day test was:                                                                                                                                                                         
> https://download.01.org/0day-ci/archive/20231120/202311201431.57aae8f3-oliver.sang@intel.com/config-6.6.0-rc2-00030-g423f75083acd                                                           
> 
> It has CONFIG_CFS_BANDWIDTH=y but CONFIG_NUMA_BALANCING is not set so
> this was not the culprit, but that configuration could have been a
> cause if it was possible.
> 
> In the same future-not-upstream commit kernel/sched/core.c sched_core_sysctls
> can be empty if you don't have the following:                                                                                                                     
> 
> CONFIG_SCHEDSTATS       --> not set on 0-day kernel
> CONFIG_UCLAMP_TASK      --> not set on 0-day kernel                                                                                                                                         
> CONFIG_NUMA_BALANCING   --> not set on 0-day kernel
> 
> So that was the cause, and an example real valid config which can trigger
> a crash. But that patch was never upstream.
> 
> Now, we can look for existing removal of sysctl sentinels with:
> 
> git log -p --grep "superfluous sentinel element"
> 
> And of these, at first glance, only locks_sysctls seems like it *could*
> possibly end up in a situation where random config would create an empty
> sysctl, but exanding on the code we see that's not possible because
> leases-enable sysctl entry is always built if you have sysctls enabled:
> 
> static struct ctl_table llocks_sysctlsocks_sysctls[] = {
>         {
>                 .procname       = "leases-enable",
>                 .data           = &leases_enable,
>                 .maxlen         = sizeof(int),                                   
>                 .mode           = 0644,
>                 .proc_handler   = proc_dointvec,
>         },
> #ifdef CONFIG_MMU
>         {
>                 .procname       = "lease-break-time",
>                 .data           = &lease_break_time,                             
>                 .maxlen         = sizeof(int),
>                 .mode           = 0644,
>                 .proc_handler   = proc_dointvec,
>         },
> #endif /* CONFIG_MMU */
> };
> 
> The out of bounds fix commit should have just had the tag:
> 
> Fixes 9edbfe92a0a13 ("sysctl: Add size to register_sysctl")
> 
> Backporting this is fine, but wouldn't fix an issue unless an external
It is not only fine, I think it is necessary to avoid some other future
backported patch actually introducing an empty sysctl.

> module had empty sysctls. And exploiting this is not possible unless
> you purposely build an external module which could end up with empty
> sysctls.
This is exactly my conclusion.
Very nice summary Luis. Thx for putting it together

> 
> HTH,
> 
>   Luis


-- 

Joel Granados

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 659 bytes --]

Re: CVE-2023-52596: sysctl: Fix out of bounds access for empty sysctl registers

[Luis Chamberlain]

On Tue, Mar 12, 2024 at 10:45:28AM +0100, Michal Hocko wrote:
> On Tue 12-03-24 09:17:30, Lee Jones wrote:
> [...]
> > > Backporting this is fine, but wouldn't fix an issue unless an external
> > > module had empty sysctls. And exploiting this is not possible unless
> > > you purposely build an external module which could end up with empty
> > > sysctls.
> 
> Thanks for the clarification Luis!
> 
> > Thanks for the amazing explanation Luis.
> > 
> > If I'm reading this correctly, an issue does exist, but an attacker
> > would have to lay some foundations before it could be triggered.  Sounds
> > like loading of a malicious or naive module would be enough.
> 
> If the bar is set as high as a kernel module to create and empty sysctl
> directory then I think it is safe to say that the security aspect is
> mostly moot. There are much simpler ways to attack the system if you are
> able to load a kernel module.

Indeed, a simple BUG_ON(1) on external modules cannot possible be a
source of a CVE. And so this becomes BUG_ON(when_sysctl_empty()) where
when_sysctl_empty() is hypotethical and I think the source of this
question for CVE. Today's that not at boot time or dynamically with any
linux kernel sources released, and so its only possible if:

  a) As Joel indicated if you backported an empty sysctl array
     (which would be unless you carried all the infrastructure to support it).

  b) an external module has an empty sysctl

HTH.

  Luis

Re: CVE-2023-52596: sysctl: Fix out of bounds access for empty sysctl registers

[Lee Jones]

On Tue, 12 Mar 2024, Luis Chamberlain wrote:

> On Tue, Mar 12, 2024 at 10:45:28AM +0100, Michal Hocko wrote:
> > On Tue 12-03-24 09:17:30, Lee Jones wrote:
> > [...]
> > > > Backporting this is fine, but wouldn't fix an issue unless an external
> > > > module had empty sysctls. And exploiting this is not possible unless
> > > > you purposely build an external module which could end up with empty
> > > > sysctls.
> > 
> > Thanks for the clarification Luis!
> > 
> > > Thanks for the amazing explanation Luis.
> > > 
> > > If I'm reading this correctly, an issue does exist, but an attacker
> > > would have to lay some foundations before it could be triggered.  Sounds
> > > like loading of a malicious or naive module would be enough.
> > 
> > If the bar is set as high as a kernel module to create and empty sysctl
> > directory then I think it is safe to say that the security aspect is
> > mostly moot. There are much simpler ways to attack the system if you are
> > able to load a kernel module.
> 
> Indeed, a simple BUG_ON(1) on external modules cannot possible be a
> source of a CVE. And so this becomes BUG_ON(when_sysctl_empty()) where

Issues that are capable of crashing the kernel in any way, including
with WARN() or BUG() are being considered weaknesses and presently get
CVEs.

> when_sysctl_empty() is hypotethical and I think the source of this
> question for CVE. Today's that not at boot time or dynamically with
> any linux kernel sources released, and so its only possible if:
> 
>   a) As Joel indicated if you backported an empty sysctl array (which
>   would be unless you carried all the infrastructure to support it).
> 
>   b) an external module has an empty sysctl

So what we're discussing here is weather this situation is
_possible_, however unlikely.

You are the maintainer here, so the final decision is yours.  If you say
this situation is impossible and the CVE should be revoked, I'll go
ahead and do just that.

-- 
Lee Jones [李琼斯]

Re: CVE-2023-52596: sysctl: Fix out of bounds access for empty sysctl registers

[Luis Chamberlain]

+ Kees,

On Tue, Mar 12, 2024 at 03:49:10PM +0000, Lee Jones wrote:
> On Tue, 12 Mar 2024, Luis Chamberlain wrote:
> 
> > On Tue, Mar 12, 2024 at 10:45:28AM +0100, Michal Hocko wrote:
> > > On Tue 12-03-24 09:17:30, Lee Jones wrote:
> > > [...]
> > > > > Backporting this is fine, but wouldn't fix an issue unless an external
> > > > > module had empty sysctls. And exploiting this is not possible unless
> > > > > you purposely build an external module which could end up with empty
> > > > > sysctls.
> > > 
> > > Thanks for the clarification Luis!
> > > 
> > > > Thanks for the amazing explanation Luis.
> > > > 
> > > > If I'm reading this correctly, an issue does exist, but an attacker
> > > > would have to lay some foundations before it could be triggered.  Sounds
> > > > like loading of a malicious or naive module would be enough.
> > > 
> > > If the bar is set as high as a kernel module to create and empty sysctl
> > > directory then I think it is safe to say that the security aspect is
> > > mostly moot. There are much simpler ways to attack the system if you are
> > > able to load a kernel module.
> > 
> > Indeed, a simple BUG_ON(1) on external modules cannot possible be a
> > source of a CVE. And so this becomes BUG_ON(when_sysctl_empty()) where
> 
> Issues that are capable of crashing the kernel in any way, including
> with WARN() or BUG() are being considered weaknesses and presently get
> CVEs.

Its not possible to crash any released kernel with the out of bounds issue
today, the commit is just a fix for a future world with empty sysctls
which just don't exist today.

Yes you can crash an external module with an empty sysctl on kernels
without that commit, but an empty sysctl is not common practice for
external modules, they'd have to have custom #ifdefs embedded as noted
earlier with the example crash. So your average external module should
not be able to crash existing kernels. The scope of a crash then would
be external modules that used older kernels without the fix starting after
v6.6. Since the fix is already meged on v6.6+ the scope of possible
kernels is small, and you'd need a specially crafted sysctl empty array
to do so.

> > when_sysctl_empty() is hypotethical and I think the source of this
> > question for CVE. Today's that not at boot time or dynamically with
> > any linux kernel sources released, and so its only possible if:
> > 
> >   a) As Joel indicated if you backported an empty sysctl array (which
> >   would be unless you carried all the infrastructure to support it).
> > 
> >   b) an external module has an empty sysctl
> 
> So what we're discussing here is weather this situation is
> _possible_, however unlikely.

I tried my best to summarize that world as we see it above.

> You are the maintainer here, so the final decision is yours.  If you say
> this situation is impossible and the CVE should be revoked, I'll go
> ahead and do just that.

To the best of our ability, from our perspective, upon our review, the
only way to trigger a crash would be with sysctls on external modules
loaded on these kernels:

 * v6.6 up to v6.6.15 (v6.6.16 has the fix backported) so 16 releases
 * v6.7 up to v6.7.3  (v6.7.4 has the fix backported) so 4 releases

External modules having empty sysctls should be rare, however possible.
So these 20 release would be affected by a crash with specially crafted
external modules. I suppose one way to exploit this, might be for a
vendor providing an external safe-looking module with #ifdefs which make
a sysctl seem non-empty but in reality it would be. That issue would
exist for 20 kernel releases. Could someone craft something with the out
of bounds access given the context to do something evil? Your domain of
expertise, your call, not ours.

  Luis

Re: CVE-2023-52596: sysctl: Fix out of bounds access for empty sysctl registers

[Kees Cook]

On Tue, Mar 12, 2024 at 11:04:09AM -0700, Luis Chamberlain wrote:
> + Kees,
> 
> On Tue, Mar 12, 2024 at 03:49:10PM +0000, Lee Jones wrote:
> > On Tue, 12 Mar 2024, Luis Chamberlain wrote:
> > 
> > > On Tue, Mar 12, 2024 at 10:45:28AM +0100, Michal Hocko wrote:
> > > > On Tue 12-03-24 09:17:30, Lee Jones wrote:
> > > > [...]
> > > > > > Backporting this is fine, but wouldn't fix an issue unless an external
> > > > > > module had empty sysctls. And exploiting this is not possible unless
> > > > > > you purposely build an external module which could end up with empty
> > > > > > sysctls.
> > > > 
> > > > Thanks for the clarification Luis!
> > > > 
> > > > > Thanks for the amazing explanation Luis.
> > > > > 
> > > > > If I'm reading this correctly, an issue does exist, but an attacker
> > > > > would have to lay some foundations before it could be triggered.  Sounds
> > > > > like loading of a malicious or naive module would be enough.
> > > > 
> > > > If the bar is set as high as a kernel module to create and empty sysctl
> > > > directory then I think it is safe to say that the security aspect is
> > > > mostly moot. There are much simpler ways to attack the system if you are
> > > > able to load a kernel module.
> > > 
> > > Indeed, a simple BUG_ON(1) on external modules cannot possible be a
> > > source of a CVE. And so this becomes BUG_ON(when_sysctl_empty()) where
> > 
> > Issues that are capable of crashing the kernel in any way, including
> > with WARN() or BUG() are being considered weaknesses and presently get
> > CVEs.
> 
> Its not possible to crash any released kernel with the out of bounds issue
> today, the commit is just a fix for a future world with empty sysctls
> which just don't exist today.
> 
> Yes you can crash an external module with an empty sysctl on kernels
> without that commit, but an empty sysctl is not common practice for
> external modules, they'd have to have custom #ifdefs embedded as noted
> earlier with the example crash. So your average external module should
> not be able to crash existing kernels. The scope of a crash then would
> be external modules that used older kernels without the fix starting after
> v6.6. Since the fix is already meged on v6.6+ the scope of possible
> kernels is small, and you'd need a specially crafted sysctl empty array
> to do so.
> 
> > > when_sysctl_empty() is hypotethical and I think the source of this
> > > question for CVE. Today's that not at boot time or dynamically with
> > > any linux kernel sources released, and so its only possible if:
> > > 
> > >   a) As Joel indicated if you backported an empty sysctl array (which
> > >   would be unless you carried all the infrastructure to support it).
> > > 
> > >   b) an external module has an empty sysctl
> > 
> > So what we're discussing here is weather this situation is
> > _possible_, however unlikely.
> 
> I tried my best to summarize that world as we see it above.
> 
> > You are the maintainer here, so the final decision is yours.  If you say
> > this situation is impossible and the CVE should be revoked, I'll go
> > ahead and do just that.
> 
> To the best of our ability, from our perspective, upon our review, the
> only way to trigger a crash would be with sysctls on external modules
> loaded on these kernels:
> 
>  * v6.6 up to v6.6.15 (v6.6.16 has the fix backported) so 16 releases
>  * v6.7 up to v6.7.3  (v6.7.4 has the fix backported) so 4 releases
> 
> External modules having empty sysctls should be rare, however possible.
> So these 20 release would be affected by a crash with specially crafted
> external modules. I suppose one way to exploit this, might be for a
> vendor providing an external safe-looking module with #ifdefs which make
> a sysctl seem non-empty but in reality it would be. That issue would
> exist for 20 kernel releases. Could someone craft something with the out
> of bounds access given the context to do something evil? Your domain of
> expertise, your call, not ours.

I'm not a member of the CNA, but I would lean "yes, the absolute weakest
of CVE" after spending some time reading the code, reading this thread,
to dig in and look at this. If it's a malicious module, it doesn't matter:
the module can do anything. If it's a published module that an attacker
could use due to the resulting logic of processing the 0th sysctl table
entry, okay, yes, CVE. Likely insanely rare, but not impossible. But,
if, as Luis says, there are no upstream modules like this, then it's
not a CVE.

So for real-world impact, we'd have to either say "there might be an
out-of-tree module that could be used as a stepping stone here, and we
want to protect our users, so let's assign a CVE" or we take a hard line
and say that's up to downstreams to assign CVEs for their modules.

I have tried to argue before that it's up to the core kernel code to Do
The Right Thing, even in the face of crappy out-of-tree code, so to me,
since this is a (very very very limited) weakness in the core kernel
code, give it a CVE.

My attempt at a CVSS for it yields a 3.4 overall:
AV:L/AC:H/PR:H/UI:N/S:U/C:L/I:L/A:L/E:U/RL:O/RC:X
https://nvd.nist.gov/vuln-metrics/cvss/v3-calculator?vector=AV:L/AC:H/PR:H/UI:N/S:U/C:L/I:L/A:L/E:U/RL:O/RC:X&version=3.1

-- 
Kees Cook

Re: CVE-2023-52596: sysctl: Fix out of bounds access for empty sysctl registers

[Lee Jones]

On Tue, 12 Mar 2024, Kees Cook wrote:

> On Tue, Mar 12, 2024 at 11:04:09AM -0700, Luis Chamberlain wrote:
> > + Kees,
> > 
> > On Tue, Mar 12, 2024 at 03:49:10PM +0000, Lee Jones wrote:
> > > On Tue, 12 Mar 2024, Luis Chamberlain wrote:
> > > 
> > > > On Tue, Mar 12, 2024 at 10:45:28AM +0100, Michal Hocko wrote:
> > > > > On Tue 12-03-24 09:17:30, Lee Jones wrote:
> > > > > [...]
> > > > > > > Backporting this is fine, but wouldn't fix an issue unless an external
> > > > > > > module had empty sysctls. And exploiting this is not possible unless
> > > > > > > you purposely build an external module which could end up with empty
> > > > > > > sysctls.
> > > > > 
> > > > > Thanks for the clarification Luis!
> > > > > 
> > > > > > Thanks for the amazing explanation Luis.
> > > > > > 
> > > > > > If I'm reading this correctly, an issue does exist, but an attacker
> > > > > > would have to lay some foundations before it could be triggered.  Sounds
> > > > > > like loading of a malicious or naive module would be enough.
> > > > > 
> > > > > If the bar is set as high as a kernel module to create and empty sysctl
> > > > > directory then I think it is safe to say that the security aspect is
> > > > > mostly moot. There are much simpler ways to attack the system if you are
> > > > > able to load a kernel module.
> > > > 
> > > > Indeed, a simple BUG_ON(1) on external modules cannot possible be a
> > > > source of a CVE. And so this becomes BUG_ON(when_sysctl_empty()) where
> > > 
> > > Issues that are capable of crashing the kernel in any way, including
> > > with WARN() or BUG() are being considered weaknesses and presently get
> > > CVEs.
> > 
> > Its not possible to crash any released kernel with the out of bounds issue
> > today, the commit is just a fix for a future world with empty sysctls
> > which just don't exist today.
> > 
> > Yes you can crash an external module with an empty sysctl on kernels
> > without that commit, but an empty sysctl is not common practice for
> > external modules, they'd have to have custom #ifdefs embedded as noted
> > earlier with the example crash. So your average external module should
> > not be able to crash existing kernels. The scope of a crash then would
> > be external modules that used older kernels without the fix starting after
> > v6.6. Since the fix is already meged on v6.6+ the scope of possible
> > kernels is small, and you'd need a specially crafted sysctl empty array
> > to do so.
> > 
> > > > when_sysctl_empty() is hypotethical and I think the source of this
> > > > question for CVE. Today's that not at boot time or dynamically with
> > > > any linux kernel sources released, and so its only possible if:
> > > > 
> > > >   a) As Joel indicated if you backported an empty sysctl array (which
> > > >   would be unless you carried all the infrastructure to support it).
> > > > 
> > > >   b) an external module has an empty sysctl
> > > 
> > > So what we're discussing here is weather this situation is
> > > _possible_, however unlikely.
> > 
> > I tried my best to summarize that world as we see it above.
> > 
> > > You are the maintainer here, so the final decision is yours.  If you say
> > > this situation is impossible and the CVE should be revoked, I'll go
> > > ahead and do just that.
> > 
> > To the best of our ability, from our perspective, upon our review, the
> > only way to trigger a crash would be with sysctls on external modules
> > loaded on these kernels:
> > 
> >  * v6.6 up to v6.6.15 (v6.6.16 has the fix backported) so 16 releases
> >  * v6.7 up to v6.7.3  (v6.7.4 has the fix backported) so 4 releases
> > 
> > External modules having empty sysctls should be rare, however possible.
> > So these 20 release would be affected by a crash with specially crafted
> > external modules. I suppose one way to exploit this, might be for a
> > vendor providing an external safe-looking module with #ifdefs which make
> > a sysctl seem non-empty but in reality it would be. That issue would
> > exist for 20 kernel releases. Could someone craft something with the out
> > of bounds access given the context to do something evil? Your domain of
> > expertise, your call, not ours.
> 
> I'm not a member of the CNA, but I would lean "yes, the absolute weakest
> of CVE" after spending some time reading the code, reading this thread,
> to dig in and look at this. If it's a malicious module, it doesn't matter:
> the module can do anything. If it's a published module that an attacker
> could use due to the resulting logic of processing the 0th sysctl table
> entry, okay, yes, CVE. Likely insanely rare, but not impossible. But,
> if, as Luis says, there are no upstream modules like this, then it's
> not a CVE.
> 
> So for real-world impact, we'd have to either say "there might be an
> out-of-tree module that could be used as a stepping stone here, and we
> want to protect our users, so let's assign a CVE" or we take a hard line
> and say that's up to downstreams to assign CVEs for their modules.
> 
> I have tried to argue before that it's up to the core kernel code to Do
> The Right Thing, even in the face of crappy out-of-tree code, so to me,
> since this is a (very very very limited) weakness in the core kernel
> code, give it a CVE.
> 
> My attempt at a CVSS for it yields a 3.4 overall:
> AV:L/AC:H/PR:H/UI:N/S:U/C:L/I:L/A:L/E:U/RL:O/RC:X
> https://nvd.nist.gov/vuln-metrics/cvss/v3-calculator?vector=AV:L/AC:H/PR:H/UI:N/S:U/C:L/I:L/A:L/E:U/RL:O/RC:X&version=3.1

Thank you Luis and Kees for your input.  Your efforts are very much
appreciated.  I have read and digested everyone's points.

Since no one (including myself) is willing to conclude that this
represents _zero_ risk, the allocation will not be rescinded.  In our
view a CVE, however weak, is still a CVE.  Thus, inline with our
documented cautious posture I'm going to err on the side of it.

Thanks again everyone.

-- 
Lee Jones [李琼斯]

Re: CVE-2023-52596: sysctl: Fix out of bounds access for empty sysctl registers

[Michal Hocko]

On Tue 12-03-24 14:47:59, Kees Cook wrote:
> On Tue, Mar 12, 2024 at 11:04:09AM -0700, Luis Chamberlain wrote:
> > + Kees,
> > 
> > On Tue, Mar 12, 2024 at 03:49:10PM +0000, Lee Jones wrote:
> > > On Tue, 12 Mar 2024, Luis Chamberlain wrote:
> > > 
> > > > On Tue, Mar 12, 2024 at 10:45:28AM +0100, Michal Hocko wrote:
> > > > > On Tue 12-03-24 09:17:30, Lee Jones wrote:
> > > > > [...]
> > > > > > > Backporting this is fine, but wouldn't fix an issue unless an external
> > > > > > > module had empty sysctls. And exploiting this is not possible unless
> > > > > > > you purposely build an external module which could end up with empty
> > > > > > > sysctls.
> > > > > 
> > > > > Thanks for the clarification Luis!
> > > > > 
> > > > > > Thanks for the amazing explanation Luis.
> > > > > > 
> > > > > > If I'm reading this correctly, an issue does exist, but an attacker
> > > > > > would have to lay some foundations before it could be triggered.  Sounds
> > > > > > like loading of a malicious or naive module would be enough.
> > > > > 
> > > > > If the bar is set as high as a kernel module to create and empty sysctl
> > > > > directory then I think it is safe to say that the security aspect is
> > > > > mostly moot. There are much simpler ways to attack the system if you are
> > > > > able to load a kernel module.
> > > > 
> > > > Indeed, a simple BUG_ON(1) on external modules cannot possible be a
> > > > source of a CVE. And so this becomes BUG_ON(when_sysctl_empty()) where
> > > 
> > > Issues that are capable of crashing the kernel in any way, including
> > > with WARN() or BUG() are being considered weaknesses and presently get
> > > CVEs.
> > 
> > Its not possible to crash any released kernel with the out of bounds issue
> > today, the commit is just a fix for a future world with empty sysctls
> > which just don't exist today.
> > 
> > Yes you can crash an external module with an empty sysctl on kernels
> > without that commit, but an empty sysctl is not common practice for
> > external modules, they'd have to have custom #ifdefs embedded as noted
> > earlier with the example crash. So your average external module should
> > not be able to crash existing kernels. The scope of a crash then would
> > be external modules that used older kernels without the fix starting after
> > v6.6. Since the fix is already meged on v6.6+ the scope of possible
> > kernels is small, and you'd need a specially crafted sysctl empty array
> > to do so.
> > 
> > > > when_sysctl_empty() is hypotethical and I think the source of this
> > > > question for CVE. Today's that not at boot time or dynamically with
> > > > any linux kernel sources released, and so its only possible if:
> > > > 
> > > >   a) As Joel indicated if you backported an empty sysctl array (which
> > > >   would be unless you carried all the infrastructure to support it).
> > > > 
> > > >   b) an external module has an empty sysctl
> > > 
> > > So what we're discussing here is weather this situation is
> > > _possible_, however unlikely.
> > 
> > I tried my best to summarize that world as we see it above.
> > 
> > > You are the maintainer here, so the final decision is yours.  If you say
> > > this situation is impossible and the CVE should be revoked, I'll go
> > > ahead and do just that.
> > 
> > To the best of our ability, from our perspective, upon our review, the
> > only way to trigger a crash would be with sysctls on external modules
> > loaded on these kernels:
> > 
> >  * v6.6 up to v6.6.15 (v6.6.16 has the fix backported) so 16 releases
> >  * v6.7 up to v6.7.3  (v6.7.4 has the fix backported) so 4 releases
> > 
> > External modules having empty sysctls should be rare, however possible.
> > So these 20 release would be affected by a crash with specially crafted
> > external modules. I suppose one way to exploit this, might be for a
> > vendor providing an external safe-looking module with #ifdefs which make
> > a sysctl seem non-empty but in reality it would be. That issue would
> > exist for 20 kernel releases. Could someone craft something with the out
> > of bounds access given the context to do something evil? Your domain of
> > expertise, your call, not ours.
> 
> I'm not a member of the CNA, but I would lean "yes, the absolute weakest
> of CVE" after spending some time reading the code, reading this thread,
> to dig in and look at this. If it's a malicious module, it doesn't matter:
> the module can do anything. If it's a published module that an attacker
> could use due to the resulting logic of processing the 0th sysctl table
> entry, okay, yes, CVE. Likely insanely rare, but not impossible. But,
> if, as Luis says, there are no upstream modules like this, then it's
> not a CVE.
> 
> So for real-world impact, we'd have to either say "there might be an
> out-of-tree module that could be used as a stepping stone here, and we
> want to protect our users, so let's assign a CVE" or we take a hard line
> and say that's up to downstreams to assign CVEs for their modules.
> 
> I have tried to argue before that it's up to the core kernel code to Do
> The Right Thing, even in the face of crappy out-of-tree code, so to me,
> since this is a (very very very limited) weakness in the core kernel
> code, give it a CVE.

To be really honest I find this reasoning very weak. Our internal APIs
consumable by kernel modules are by large not defending against invalid
or unsupported inputs. This is just yet another of those that have been
fixed which is good for future kernel robustnes but I do not really get
why should we as a kernel community care about out-of-tree modules as
kernels with those loaded are unsupported anyway.

> My attempt at a CVSS for it yields a 3.4 overall:
> AV:L/AC:H/PR:H/UI:N/S:U/C:L/I:L/A:L/E:U/RL:O/RC:X
> https://nvd.nist.gov/vuln-metrics/cvss/v3-calculator?vector=AV:L/AC:H/PR:H/UI:N/S:U/C:L/I:L/A:L/E:U/RL:O/RC:X&version=3.1
-- 
Michal Hocko
SUSE Labs

Re: CVE-2023-52596: sysctl: Fix out of bounds access for empty sysctl registers

[Pavel Machek]

[-- Attachment #1: Type: text/plain, Size: 1890 bytes --]

Hi!

> > I have tried to argue before that it's up to the core kernel code to Do
> > The Right Thing, even in the face of crappy out-of-tree code, so to me,
> > since this is a (very very very limited) weakness in the core kernel
> > code, give it a CVE.
> > 
> > My attempt at a CVSS for it yields a 3.4 overall:
> > AV:L/AC:H/PR:H/UI:N/S:U/C:L/I:L/A:L/E:U/RL:O/RC:X
> > https://nvd.nist.gov/vuln-metrics/cvss/v3-calculator?vector=AV:L/AC:H/PR:H/UI:N/S:U/C:L/I:L/A:L/E:U/RL:O/RC:X&version=3.1
> 
> Thank you Luis and Kees for your input.  Your efforts are very much
> appreciated.  I have read and digested everyone's points.
> 
> Since no one (including myself) is willing to conclude that this
> represents _zero_ risk, the allocation will not be rescinded.  In our

Well, if you insist this is real risk (it is not) would you be so kind
at at least fix the "vulnerability" description?

"Module can trigger BUG_ON in kernel" would be suitable, according to
the discussion. Current description is copy/paste nonsense :-(.

Best regards,
								Pavel

https://nvd.nist.gov/vuln/detail/CVE-2023-52596
Description
In the Linux kernel, the following vulnerability has been resolved:
sysctl: Fix out of bounds access for empty sysctl registers When
registering tables to the sysctl subsystem there is a check to see if
header is a permanently empty directory (used for mounts). This check
evaluates the first element of the ctl_table. This results in an out
of bounds evaluation when registering empty directories. The function
register_sysctl_mount_point now passes a ctl_table of size 1 instead
of size 0. It now relies solely on the type to identify a permanently
empty register. Make sure that the ctl_table has at least one element
before testing for permanent emptiness.


-- 
People of Russia, stop Putin before his war on Ukraine escalates.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 195 bytes --]