Re: CVE-2024-40938: landlock: Fix d_parent walk

Re: CVE-2024-40938: landlock: Fix d_parent walk

[Mickaël Salaün]

Hello,

AFAIK, commit 88da52ccd66e ("landlock: Fix d_parent walk") doesn't fix a
security issue but an unexpected case.  The triggered WARN_ON_ONCE() is
just a canary, and this case was correctly handled with defensive
programming and didn't allow to bypass the security policy nor to harm
the kernel.  However, this fix should indeed be backported.

Could you please Cc me for future CVE related to my changes or to
Landlock?  For kernel CVEs, I think it would be good to Cc at least
maintainers, reviewers, authors, and committers for the related commits.

Regards,
 Mickaël

On Fri, Jul 12, 2024 at 02:27:36PM +0200, Greg Kroah-Hartman wrote:
> Description
> ===========
> 
> In the Linux kernel, the following vulnerability has been resolved:
> 
> landlock: Fix d_parent walk
> 
> The WARN_ON_ONCE() in collect_domain_accesses() can be triggered when
> trying to link a root mount point.  This cannot work in practice because
> this directory is mounted, but the VFS check is done after the call to
> security_path_link().
> 
> Do not use source directory's d_parent when the source directory is the
> mount point.
> 
> [mic: Fix commit message]
> 
> The Linux kernel CVE team has assigned CVE-2024-40938 to this issue.
> 
> 
> Affected and fixed versions
> ===========================
> 
> 	Issue introduced in 5.19 with commit b91c3e4ea756 and fixed in 6.1.95 with commit b6e5e6964358
> 	Issue introduced in 5.19 with commit b91c3e4ea756 and fixed in 6.6.35 with commit cc30d05b34f9
> 	Issue introduced in 5.19 with commit b91c3e4ea756 and fixed in 6.9.6 with commit c7618c7b0b8c
> 	Issue introduced in 5.19 with commit b91c3e4ea756 and fixed in 6.10-rc2 with commit 88da52ccd66e
> 
> Please see https://www.kernel.org for a full list of currently supported
> kernel versions by the kernel community.
> 
> Unaffected versions might change over time as fixes are backported to
> older supported kernel versions.  The official CVE entry at
> 	https://cve.org/CVERecord/?id=CVE-2024-40938
> will be updated if fixes are backported, please check that for the most
> up to date information about this issue.
> 
> 
> Affected files
> ==============
> 
> The file(s) affected by this issue are:
> 	security/landlock/fs.c
> 
> 
> Mitigation
> ==========
> 
> The Linux kernel CVE team recommends that you update to the latest
> stable kernel version for this, and many other bugfixes.  Individual
> changes are never tested alone, but rather are part of a larger kernel
> release.  Cherry-picking individual commits is not recommended or
> supported by the Linux kernel community at all.  If however, updating to
> the latest release is impossible, the individual changes to resolve this
> issue can be found at these commits:
> 	https://git.kernel.org/stable/c/b6e5e696435832b33e40775f060ef5c95f4fda1f
> 	https://git.kernel.org/stable/c/cc30d05b34f9a087a6928d09b131f7b491e9ab11
> 	https://git.kernel.org/stable/c/c7618c7b0b8c45bcef34410cc1d1e953eb17f8f6
> 	https://git.kernel.org/stable/c/88da52ccd66e65f2e63a6c35c9dff55d448ef4dc

Re: CVE-2024-40938: landlock: Fix d_parent walk

[Greg Kroah-Hartman]

On Mon, Jul 15, 2024 at 12:37:53PM +0200, Mickaël Salaün wrote:
> Hello,
> 
> AFAIK, commit 88da52ccd66e ("landlock: Fix d_parent walk") doesn't fix a
> security issue but an unexpected case.  The triggered WARN_ON_ONCE() is
> just a canary, and this case was correctly handled with defensive
> programming and didn't allow to bypass the security policy nor to harm
> the kernel.  However, this fix should indeed be backported.

If a WARN_ON() is hit, a machine with panic_on_warn enabled will reboot,
hence if there is any way that userspace can hit this, it needs to be
issued a CVE, sorry.

> Could you please Cc me for future CVE related to my changes or to
> Landlock?  For kernel CVEs, I think it would be good to Cc at least
> maintainers, reviewers, authors, and committers for the related commits.

I suggest setting up lei to watch the linux-cve-announce mailing list if
you wish to do this (just filter for landlock stuff).  Automatically
mailing cve stuff to maintainers has been deemed too "noisy" which is
why we do not do this by default.

thanks,

greg k-h

Re: CVE-2024-40938: landlock: Fix d_parent walk

[Mickaël Salaün]

On Mon, Jul 15, 2024 at 01:16:38PM +0200, Greg Kroah-Hartman wrote:
> On Mon, Jul 15, 2024 at 12:37:53PM +0200, Mickaël Salaün wrote:
> > Hello,
> > 
> > AFAIK, commit 88da52ccd66e ("landlock: Fix d_parent walk") doesn't fix a
> > security issue but an unexpected case.  The triggered WARN_ON_ONCE() is
> > just a canary, and this case was correctly handled with defensive
> > programming and didn't allow to bypass the security policy nor to harm
> > the kernel.  However, this fix should indeed be backported.
> 
> If a WARN_ON() is hit, a machine with panic_on_warn enabled will reboot,
> hence if there is any way that userspace can hit this, it needs to be
> issued a CVE, sorry.

OK, I didn't know about this panic_on_warn rule for CVE.  Out of
curiosity, panic_on_warn is definitely useful for fuzzing and testing,
but what is the rational to enable panic_on_warn on production systems?
It literally transforms a warning message into a system DoS (i.e.
WARN_ON into BUG_ON).  We should explicitly use BUG_ON() if this is a
critical unhandled case, right?

> 
> > Could you please Cc me for future CVE related to my changes or to
> > Landlock?  For kernel CVEs, I think it would be good to Cc at least
> > maintainers, reviewers, authors, and committers for the related commits.
> 
> I suggest setting up lei to watch the linux-cve-announce mailing list if
> you wish to do this (just filter for landlock stuff).  Automatically
> mailing cve stuff to maintainers has been deemed too "noisy" which is
> why we do not do this by default.

Well, it might be too noisy for some but I guess/hope not for most.
Email filtering should be easy for the few receiving too many of these
emails though.

Re: CVE-2024-40938: landlock: Fix d_parent walk

[Greg Kroah-Hartman]

On Mon, Jul 15, 2024 at 02:20:59PM +0200, Mickaël Salaün wrote:
> On Mon, Jul 15, 2024 at 01:16:38PM +0200, Greg Kroah-Hartman wrote:
> > On Mon, Jul 15, 2024 at 12:37:53PM +0200, Mickaël Salaün wrote:
> > > Hello,
> > > 
> > > AFAIK, commit 88da52ccd66e ("landlock: Fix d_parent walk") doesn't fix a
> > > security issue but an unexpected case.  The triggered WARN_ON_ONCE() is
> > > just a canary, and this case was correctly handled with defensive
> > > programming and didn't allow to bypass the security policy nor to harm
> > > the kernel.  However, this fix should indeed be backported.
> > 
> > If a WARN_ON() is hit, a machine with panic_on_warn enabled will reboot,
> > hence if there is any way that userspace can hit this, it needs to be
> > issued a CVE, sorry.
> 
> OK, I didn't know about this panic_on_warn rule for CVE.  Out of
> curiosity, panic_on_warn is definitely useful for fuzzing and testing,
> but what is the rational to enable panic_on_warn on production systems?

People like to have their devices/servers rebooted if _anything_ is seen
to go wrong.  A few billion phones have this enabled, as do most all
cloud servers it seems :(

> It literally transforms a warning message into a system DoS (i.e.
> WARN_ON into BUG_ON).

Yes :(

> We should explicitly use BUG_ON() if this is a critical unhandled
> case, right?

Personally I would use neither, handle the error properly and clean up
correctly.  Only if the continuing to run would cause unrecoverable harm
(i.e. data loss or corruption or compromise) would I ever call BUG_ON().

> > > Could you please Cc me for future CVE related to my changes or to
> > > Landlock?  For kernel CVEs, I think it would be good to Cc at least
> > > maintainers, reviewers, authors, and committers for the related commits.
> > 
> > I suggest setting up lei to watch the linux-cve-announce mailing list if
> > you wish to do this (just filter for landlock stuff).  Automatically
> > mailing cve stuff to maintainers has been deemed too "noisy" which is
> > why we do not do this by default.
> 
> Well, it might be too noisy for some but I guess/hope not for most.
> Email filtering should be easy for the few receiving too many of these
> emails though.

For now we have decided to not cc: maintainers as these are all sent to
a public list that can be subscribed to if they wish to.

thanks,

greg k-h

Re: CVE-2024-40938: landlock: Fix d_parent walk

[Kees Cook]

On Mon, Jul 15, 2024 at 02:20:59PM +0200, Mickaël Salaün wrote:
> On Mon, Jul 15, 2024 at 01:16:38PM +0200, Greg Kroah-Hartman wrote:
> > On Mon, Jul 15, 2024 at 12:37:53PM +0200, Mickaël Salaün wrote:
> > > Hello,
> > > 
> > > AFAIK, commit 88da52ccd66e ("landlock: Fix d_parent walk") doesn't fix a
> > > security issue but an unexpected case.  The triggered WARN_ON_ONCE() is
> > > just a canary, and this case was correctly handled with defensive
> > > programming and didn't allow to bypass the security policy nor to harm
> > > the kernel.  However, this fix should indeed be backported.
> > 
> > If a WARN_ON() is hit, a machine with panic_on_warn enabled will reboot,
> > hence if there is any way that userspace can hit this, it needs to be
> > issued a CVE, sorry.
> 
> OK, I didn't know about this panic_on_warn rule for CVE.  Out of
> curiosity, panic_on_warn is definitely useful for fuzzing and testing,
> but what is the rational to enable panic_on_warn on production systems?
> It literally transforms a warning message into a system DoS (i.e.
> WARN_ON into BUG_ON).  We should explicitly use BUG_ON() if this is a
> critical unhandled case, right?

We need a way to raise WARN to panic for deployments that have tested
their workloads and want FORTIFY_SOURCE and UBSAN_BOUNDS to actually
perform mitigations instead of just warning. Linus rejected all prior
knobs for this and panic_on_warn (or better yet, kernel.warn_limit
syscall) is used for this purpose.

Userspace actions must never be able to reach a WARN or BUG state:
https://docs.kernel.org/process/deprecated.html#bug-and-bug-on

-- 
Kees Cook

Re: CVE-2024-40938: landlock: Fix d_parent walk

[Mickaël Salaün]

On Mon, Jul 15, 2024 at 09:11:35AM -0700, Kees Cook wrote:
> On Mon, Jul 15, 2024 at 02:20:59PM +0200, Mickaël Salaün wrote:
> > On Mon, Jul 15, 2024 at 01:16:38PM +0200, Greg Kroah-Hartman wrote:
> > > On Mon, Jul 15, 2024 at 12:37:53PM +0200, Mickaël Salaün wrote:
> > > > Hello,
> > > > 
> > > > AFAIK, commit 88da52ccd66e ("landlock: Fix d_parent walk") doesn't fix a
> > > > security issue but an unexpected case.  The triggered WARN_ON_ONCE() is
> > > > just a canary, and this case was correctly handled with defensive
> > > > programming and didn't allow to bypass the security policy nor to harm
> > > > the kernel.  However, this fix should indeed be backported.
> > > 
> > > If a WARN_ON() is hit, a machine with panic_on_warn enabled will reboot,
> > > hence if there is any way that userspace can hit this, it needs to be
> > > issued a CVE, sorry.
> > 
> > OK, I didn't know about this panic_on_warn rule for CVE.  Out of
> > curiosity, panic_on_warn is definitely useful for fuzzing and testing,
> > but what is the rational to enable panic_on_warn on production systems?
> > It literally transforms a warning message into a system DoS (i.e.
> > WARN_ON into BUG_ON).  We should explicitly use BUG_ON() if this is a
> > critical unhandled case, right?
> 
> We need a way to raise WARN to panic for deployments that have tested
> their workloads and want FORTIFY_SOURCE and UBSAN_BOUNDS to actually

OK, I guess it makes sense if we know and tested all the possible
execution paths and states, which is challenging, but I get your point.
However, for this use case, I don't see the point of promoting a WARN_ON
issue to a CVE.

> perform mitigations instead of just warning. Linus rejected all prior
> knobs for this and panic_on_warn (or better yet, kernel.warn_limit
> syscall) is used for this purpose.
> 
> Userspace actions must never be able to reach a WARN or BUG state:
> https://docs.kernel.org/process/deprecated.html#bug-and-bug-on

Yes, that's why we use WARN_ON_ONCE() to check cases that should never
happen (at the time of writting), but in practice it's useful to check
(with fuzzing) that this assertion is true.  However, if a
WARN_ON_ONCE() is reached, this doesn't mean that this is a security
issue, but just an unexpected case that kernel maintainers should be
notified with to fix it.

Re: CVE-2024-40938: landlock: Fix d_parent walk

[Kees Cook]

On Mon, Jul 15, 2024 at 08:04:21PM +0200, Mickaël Salaün wrote:
> Yes, that's why we use WARN_ON_ONCE() to check cases that should never
> happen (at the time of writting), but in practice it's useful to check
> (with fuzzing) that this assertion is true.  However, if a
> WARN_ON_ONCE() is reached, this doesn't mean that this is a security
> issue, but just an unexpected case that kernel maintainers should be
> notified with to fix it.

I leave CVE determinations to the CNA. :) I think the difficulty here is
with having no way to trivially see which WARN is security sensitive and
which isn't, and since WARNs may panic, all WARNs could be a DoS, and
therefore may be a CVE for some deployment somewhere.

-- 
Kees Cook

Re: CVE-2024-40938: landlock: Fix d_parent walk

[Greg Kroah-Hartman]

On Mon, Jul 15, 2024 at 01:17:10PM -0700, Kees Cook wrote:
> On Mon, Jul 15, 2024 at 08:04:21PM +0200, Mickaël Salaün wrote:
> > Yes, that's why we use WARN_ON_ONCE() to check cases that should never
> > happen (at the time of writting), but in practice it's useful to check
> > (with fuzzing) that this assertion is true.  However, if a
> > WARN_ON_ONCE() is reached, this doesn't mean that this is a security
> > issue, but just an unexpected case that kernel maintainers should be
> > notified with to fix it.
> 
> I leave CVE determinations to the CNA. :) I think the difficulty here is
> with having no way to trivially see which WARN is security sensitive and
> which isn't, and since WARNs may panic, all WARNs could be a DoS, and
> therefore may be a CVE for some deployment somewhere.

That is exactly correct, and why we must mark any way that userspace can
hit a WARN as needing a CVE.

thanks,

greg k-h