CVE-2024-26628: drm/amdkfd: Fix lock dependency warning

CVE-2024-26628: drm/amdkfd: Fix lock dependency warning

[Greg Kroah-Hartman]

Description
===========

In the Linux kernel, the following vulnerability has been resolved:

drm/amdkfd: Fix lock dependency warning

======================================================
WARNING: possible circular locking dependency detected
6.5.0-kfd-fkuehlin #276 Not tainted
------------------------------------------------------
kworker/8:2/2676 is trying to acquire lock:
ffff9435aae95c88 ((work_completion)(&svm_bo->eviction_work)){+.+.}-{0:0}, at: __flush_work+0x52/0x550

but task is already holding lock:
ffff9435cd8e1720 (&svms->lock){+.+.}-{3:3}, at: svm_range_deferred_list_work+0xe8/0x340 [amdgpu]

which lock already depends on the new lock.

the existing dependency chain (in reverse order) is:

-> #2 (&svms->lock){+.+.}-{3:3}:
       __mutex_lock+0x97/0xd30
       kfd_ioctl_alloc_memory_of_gpu+0x6d/0x3c0 [amdgpu]
       kfd_ioctl+0x1b2/0x5d0 [amdgpu]
       __x64_sys_ioctl+0x86/0xc0
       do_syscall_64+0x39/0x80
       entry_SYSCALL_64_after_hwframe+0x63/0xcd

-> #1 (&mm->mmap_lock){++++}-{3:3}:
       down_read+0x42/0x160
       svm_range_evict_svm_bo_worker+0x8b/0x340 [amdgpu]
       process_one_work+0x27a/0x540
       worker_thread+0x53/0x3e0
       kthread+0xeb/0x120
       ret_from_fork+0x31/0x50
       ret_from_fork_asm+0x11/0x20

-> #0 ((work_completion)(&svm_bo->eviction_work)){+.+.}-{0:0}:
       __lock_acquire+0x1426/0x2200
       lock_acquire+0xc1/0x2b0
       __flush_work+0x80/0x550
       __cancel_work_timer+0x109/0x190
       svm_range_bo_release+0xdc/0x1c0 [amdgpu]
       svm_range_free+0x175/0x180 [amdgpu]
       svm_range_deferred_list_work+0x15d/0x340 [amdgpu]
       process_one_work+0x27a/0x540
       worker_thread+0x53/0x3e0
       kthread+0xeb/0x120
       ret_from_fork+0x31/0x50
       ret_from_fork_asm+0x11/0x20

other info that might help us debug this:

Chain exists of:
  (work_completion)(&svm_bo->eviction_work) --> &mm->mmap_lock --> &svms->lock

 Possible unsafe locking scenario:

       CPU0                    CPU1
       ----                    ----
  lock(&svms->lock);
                               lock(&mm->mmap_lock);
                               lock(&svms->lock);
  lock((work_completion)(&svm_bo->eviction_work));

I believe this cannot really lead to a deadlock in practice, because
svm_range_evict_svm_bo_worker only takes the mmap_read_lock if the BO
refcount is non-0. That means it's impossible that svm_range_bo_release
is running concurrently. However, there is no good way to annotate this.

To avoid the problem, take a BO reference in
svm_range_schedule_evict_svm_bo instead of in the worker. That way it's
impossible for a BO to get freed while eviction work is pending and the
cancel_work_sync call in svm_range_bo_release can be eliminated.

v2: Use svm_bo_ref_unless_zero and explained why that's safe. Also
removed redundant checks that are already done in
amdkfd_fence_enable_signaling.

The Linux kernel CVE team has assigned CVE-2024-26628 to this issue.


Affected and fixed versions
===========================

	Fixed in 5.15.149 with commit 7a70663ba02b
	Fixed in 6.1.77 with commit 8b25d397162b
	Fixed in 6.6.16 with commit 28d2d623d2fb
	Fixed in 6.7.4 with commit cb96e492d72d
	Fixed in 6.8-rc1 with commit 47bf0f83fc86

Please see https://www.kernel.org or 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-26628
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:
	drivers/gpu/drm/amd/amdkfd/kfd_svm.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/7a70663ba02bd4e19aea8d70c979eb3bd03d839d
	https://git.kernel.org/stable/c/8b25d397162b0316ceda40afaa63ee0c4a97d28b
	https://git.kernel.org/stable/c/28d2d623d2fbddcca5c24600474e92f16ebb3a05
	https://git.kernel.org/stable/c/cb96e492d72d143d57db2d2bc143a1cee8741807
	https://git.kernel.org/stable/c/47bf0f83fc86df1bf42b385a91aadb910137c5c9

Re: CVE-2024-26628: drm/amdkfd: Fix lock dependency warning

[Michal Hocko]

On Wed 06-03-24 06:46:11, Greg KH wrote:
[...]
>  Possible unsafe locking scenario:
> 
>        CPU0                    CPU1
>        ----                    ----
>   lock(&svms->lock);
>                                lock(&mm->mmap_lock);
>                                lock(&svms->lock);
>   lock((work_completion)(&svm_bo->eviction_work));
> 
> I believe this cannot really lead to a deadlock in practice, because
> svm_range_evict_svm_bo_worker only takes the mmap_read_lock if the BO
> refcount is non-0. That means it's impossible that svm_range_bo_release
> is running concurrently. However, there is no good way to annotate this.

OK, so is this even a bug (not to mention a security/weakness)?
-- 
Michal Hocko
SUSE Labs

Re: CVE-2024-26628: drm/amdkfd: Fix lock dependency warning

[Lee Jones]

On Fri, 08 Mar 2024, Michal Hocko wrote:

> On Wed 06-03-24 06:46:11, Greg KH wrote:
> [...]
> >  Possible unsafe locking scenario:
> > 
> >        CPU0                    CPU1
> >        ----                    ----
> >   lock(&svms->lock);
> >                                lock(&mm->mmap_lock);
> >                                lock(&svms->lock);
> >   lock((work_completion)(&svm_bo->eviction_work));
> > 
> > I believe this cannot really lead to a deadlock in practice, because
> > svm_range_evict_svm_bo_worker only takes the mmap_read_lock if the BO
> > refcount is non-0. That means it's impossible that svm_range_bo_release
> > is running concurrently. However, there is no good way to annotate this.
> 
> OK, so is this even a bug (not to mention a security/weakness)?

Looks like the patch fixes a warning which can crash some kernels.  So
the CVE appears to be fixing that, rather than the impossible deadlock.

-- 
Lee Jones [李琼斯]

Re: CVE-2024-26628: drm/amdkfd: Fix lock dependency warning

[Michal Hocko]

On Thu 14-03-24 11:09:38, Lee Jones wrote:
> On Fri, 08 Mar 2024, Michal Hocko wrote:
> 
> > On Wed 06-03-24 06:46:11, Greg KH wrote:
> > [...]
> > >  Possible unsafe locking scenario:
> > > 
> > >        CPU0                    CPU1
> > >        ----                    ----
> > >   lock(&svms->lock);
> > >                                lock(&mm->mmap_lock);
> > >                                lock(&svms->lock);
> > >   lock((work_completion)(&svm_bo->eviction_work));
> > > 
> > > I believe this cannot really lead to a deadlock in practice, because
> > > svm_range_evict_svm_bo_worker only takes the mmap_read_lock if the BO
> > > refcount is non-0. That means it's impossible that svm_range_bo_release
> > > is running concurrently. However, there is no good way to annotate this.
> > 
> > OK, so is this even a bug (not to mention a security/weakness)?
> 
> Looks like the patch fixes a warning which can crash some kernels.  So
> the CVE appears to be fixing that, rather than the impossible deadlock.

Are you talking about lockdep warning or anything else?
-- 
Michal Hocko
SUSE Labs

Re: CVE-2024-26628: drm/amdkfd: Fix lock dependency warning

[Lee Jones]

On Wed, 20 Mar 2024, Michal Hocko wrote:

> On Thu 14-03-24 11:09:38, Lee Jones wrote:
> > On Fri, 08 Mar 2024, Michal Hocko wrote:
> > 
> > > On Wed 06-03-24 06:46:11, Greg KH wrote:
> > > [...]
> > > >  Possible unsafe locking scenario:
> > > > 
> > > >        CPU0                    CPU1
> > > >        ----                    ----
> > > >   lock(&svms->lock);
> > > >                                lock(&mm->mmap_lock);
> > > >                                lock(&svms->lock);
> > > >   lock((work_completion)(&svm_bo->eviction_work));
> > > > 
> > > > I believe this cannot really lead to a deadlock in practice, because
> > > > svm_range_evict_svm_bo_worker only takes the mmap_read_lock if the BO
> > > > refcount is non-0. That means it's impossible that svm_range_bo_release
> > > > is running concurrently. However, there is no good way to annotate this.
> > > 
> > > OK, so is this even a bug (not to mention a security/weakness)?
> > 
> > Looks like the patch fixes a warning which can crash some kernels.  So
> > the CVE appears to be fixing that, rather than the impossible deadlock.
> 
> Are you talking about lockdep warning or anything else?

Anything that triggers a BUG() or a WARN() (as per the splat in the
commit message).  Many in-field kernels are configured to panic on
BUG()s and WARN()s, thus triggering them are presently considered local
DoS and attract CVE status.

-- 
Lee Jones [李琼斯]

Re: CVE-2024-26628: drm/amdkfd: Fix lock dependency warning

[Lee Jones]

On Wed, 20 Mar 2024, Lee Jones wrote:

> On Wed, 20 Mar 2024, Michal Hocko wrote:
> 
> > On Thu 14-03-24 11:09:38, Lee Jones wrote:
> > > On Fri, 08 Mar 2024, Michal Hocko wrote:
> > > 
> > > > On Wed 06-03-24 06:46:11, Greg KH wrote:
> > > > [...]
> > > > >  Possible unsafe locking scenario:
> > > > > 
> > > > >        CPU0                    CPU1
> > > > >        ----                    ----
> > > > >   lock(&svms->lock);
> > > > >                                lock(&mm->mmap_lock);
> > > > >                                lock(&svms->lock);
> > > > >   lock((work_completion)(&svm_bo->eviction_work));
> > > > > 
> > > > > I believe this cannot really lead to a deadlock in practice, because
> > > > > svm_range_evict_svm_bo_worker only takes the mmap_read_lock if the BO
> > > > > refcount is non-0. That means it's impossible that svm_range_bo_release
> > > > > is running concurrently. However, there is no good way to annotate this.
> > > > 
> > > > OK, so is this even a bug (not to mention a security/weakness)?
> > > 
> > > Looks like the patch fixes a warning which can crash some kernels.  So
> > > the CVE appears to be fixing that, rather than the impossible deadlock.
> > 
> > Are you talking about lockdep warning or anything else?
> 
> Anything that triggers a BUG() or a WARN() (as per the splat in the
> commit message).  Many in-field kernels are configured to panic on
> BUG()s and WARN()s, thus triggering them are presently considered local
> DoS and attract CVE status.

We have discussed this internally and agree with your thinking.

The splat in the circular lockdep detection code appears to be generated
using some stacked pr_warn() calls, rather than a WARN().

Thus, CVE-2024-26628 has now been rejected.

  https://lore.kernel.org/all/20240320164818.3778843-2-lee@kernel.org/

Thank you for your input Michal.

-- 
Lee Jones [李琼斯]

Re: CVE-2024-26628: drm/amdkfd: Fix lock dependency warning

[Michal Hocko]

On Wed 20-03-24 16:51:27, Lee Jones wrote:
> On Wed, 20 Mar 2024, Lee Jones wrote:
> 
> > On Wed, 20 Mar 2024, Michal Hocko wrote:
> > 
> > > On Thu 14-03-24 11:09:38, Lee Jones wrote:
> > > > On Fri, 08 Mar 2024, Michal Hocko wrote:
> > > > 
> > > > > On Wed 06-03-24 06:46:11, Greg KH wrote:
> > > > > [...]
> > > > > >  Possible unsafe locking scenario:
> > > > > > 
> > > > > >        CPU0                    CPU1
> > > > > >        ----                    ----
> > > > > >   lock(&svms->lock);
> > > > > >                                lock(&mm->mmap_lock);
> > > > > >                                lock(&svms->lock);
> > > > > >   lock((work_completion)(&svm_bo->eviction_work));
> > > > > > 
> > > > > > I believe this cannot really lead to a deadlock in practice, because
> > > > > > svm_range_evict_svm_bo_worker only takes the mmap_read_lock if the BO
> > > > > > refcount is non-0. That means it's impossible that svm_range_bo_release
> > > > > > is running concurrently. However, there is no good way to annotate this.
> > > > > 
> > > > > OK, so is this even a bug (not to mention a security/weakness)?
> > > > 
> > > > Looks like the patch fixes a warning which can crash some kernels.  So
> > > > the CVE appears to be fixing that, rather than the impossible deadlock.
> > > 
> > > Are you talking about lockdep warning or anything else?
> > 
> > Anything that triggers a BUG() or a WARN() (as per the splat in the
> > commit message).  Many in-field kernels are configured to panic on
> > BUG()s and WARN()s, thus triggering them are presently considered local
> > DoS and attract CVE status.

yes I do agree that WARN() should be treated same as BUG() if
triggerable by an user (for reasons you have mentioned). Lockdep is a
different thing as you follow up below.

> We have discussed this internally and agree with your thinking.
> 
> The splat in the circular lockdep detection code appears to be generated
> using some stacked pr_warn() calls, rather than a WARN().
> 
> Thus, CVE-2024-26628 has now been rejected.
> 
>   https://lore.kernel.org/all/20240320164818.3778843-2-lee@kernel.org/
> 
> Thank you for your input Michal.

Thanks!
-- 
Michal Hocko
SUSE Labs

Re: CVE-2024-26628: drm/amdkfd: Fix lock dependency warning

[Pavel Machek]

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

On Wed 2024-03-20 15:47:34, Lee Jones wrote:
> On Wed, 20 Mar 2024, Michal Hocko wrote:
> 
> > On Thu 14-03-24 11:09:38, Lee Jones wrote:
> > > On Fri, 08 Mar 2024, Michal Hocko wrote:
> > > 
> > > > On Wed 06-03-24 06:46:11, Greg KH wrote:
> > > > [...]
> > > > >  Possible unsafe locking scenario:
> > > > > 
> > > > >        CPU0                    CPU1
> > > > >        ----                    ----
> > > > >   lock(&svms->lock);
> > > > >                                lock(&mm->mmap_lock);
> > > > >                                lock(&svms->lock);
> > > > >   lock((work_completion)(&svm_bo->eviction_work));
> > > > > 
> > > > > I believe this cannot really lead to a deadlock in practice, because
> > > > > svm_range_evict_svm_bo_worker only takes the mmap_read_lock if the BO
> > > > > refcount is non-0. That means it's impossible that svm_range_bo_release
> > > > > is running concurrently. However, there is no good way to annotate this.
> > > > 
> > > > OK, so is this even a bug (not to mention a security/weakness)?
> > > 
> > > Looks like the patch fixes a warning which can crash some kernels.  So
> > > the CVE appears to be fixing that, rather than the impossible deadlock.
> > 
> > Are you talking about lockdep warning or anything else?
> 
> Anything that triggers a BUG() or a WARN() (as per the splat in the
> commit message).  Many in-field kernels are configured to panic on
> BUG()s and WARN()s, thus triggering them are presently considered local
> DoS and attract CVE status.

So... because it is possible to configure machine to reboot on
warning, now every warning is a security issue?

Lockdep is for debugging, if someone uses it in production with panic
on reboot, they are getting exactly what they are asking for.

Not a security problem.
								Pavel
-- 
People of Russia, stop Putin before his war on Ukraine escalates.

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

Re: CVE-2024-26628: drm/amdkfd: Fix lock dependency warning

[Greg Kroah-Hartman]

On Thu, Jun 13, 2024 at 11:32:41AM +0200, Pavel Machek wrote:
> On Wed 2024-03-20 15:47:34, Lee Jones wrote:
> > On Wed, 20 Mar 2024, Michal Hocko wrote:
> > 
> > > On Thu 14-03-24 11:09:38, Lee Jones wrote:
> > > > On Fri, 08 Mar 2024, Michal Hocko wrote:
> > > > 
> > > > > On Wed 06-03-24 06:46:11, Greg KH wrote:
> > > > > [...]
> > > > > >  Possible unsafe locking scenario:
> > > > > > 
> > > > > >        CPU0                    CPU1
> > > > > >        ----                    ----
> > > > > >   lock(&svms->lock);
> > > > > >                                lock(&mm->mmap_lock);
> > > > > >                                lock(&svms->lock);
> > > > > >   lock((work_completion)(&svm_bo->eviction_work));
> > > > > > 
> > > > > > I believe this cannot really lead to a deadlock in practice, because
> > > > > > svm_range_evict_svm_bo_worker only takes the mmap_read_lock if the BO
> > > > > > refcount is non-0. That means it's impossible that svm_range_bo_release
> > > > > > is running concurrently. However, there is no good way to annotate this.
> > > > > 
> > > > > OK, so is this even a bug (not to mention a security/weakness)?
> > > > 
> > > > Looks like the patch fixes a warning which can crash some kernels.  So
> > > > the CVE appears to be fixing that, rather than the impossible deadlock.
> > > 
> > > Are you talking about lockdep warning or anything else?
> > 
> > Anything that triggers a BUG() or a WARN() (as per the splat in the
> > commit message).  Many in-field kernels are configured to panic on
> > BUG()s and WARN()s, thus triggering them are presently considered local
> > DoS and attract CVE status.
> 
> So... because it is possible to configure machine to reboot on
> warning, now every warning is a security issue?
> 
> Lockdep is for debugging, if someone uses it in production with panic
> on reboot, they are getting exactly what they are asking for.
> 
> Not a security problem.

And we agree, I don't know what you are arguing about here, please stop.

greg k-h

Re: CVE-2024-26628: drm/amdkfd: Fix lock dependency warning

[Pavel Machek]

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

On Thu 2024-06-13 12:16:50, Greg Kroah-Hartman wrote:
> On Thu, Jun 13, 2024 at 11:32:41AM +0200, Pavel Machek wrote:
> > On Wed 2024-03-20 15:47:34, Lee Jones wrote:
> > > On Wed, 20 Mar 2024, Michal Hocko wrote:
> > > 
> > > > On Thu 14-03-24 11:09:38, Lee Jones wrote:
> > > > > On Fri, 08 Mar 2024, Michal Hocko wrote:
> > > > > 
> > > > > > On Wed 06-03-24 06:46:11, Greg KH wrote:
> > > > > > [...]
> > > > > > >  Possible unsafe locking scenario:
> > > > > > > 
> > > > > > >        CPU0                    CPU1
> > > > > > >        ----                    ----
> > > > > > >   lock(&svms->lock);
> > > > > > >                                lock(&mm->mmap_lock);
> > > > > > >                                lock(&svms->lock);
> > > > > > >   lock((work_completion)(&svm_bo->eviction_work));
> > > > > > > 
> > > > > > > I believe this cannot really lead to a deadlock in practice, because
> > > > > > > svm_range_evict_svm_bo_worker only takes the mmap_read_lock if the BO
> > > > > > > refcount is non-0. That means it's impossible that svm_range_bo_release
> > > > > > > is running concurrently. However, there is no good way to annotate this.
> > > > > > 
> > > > > > OK, so is this even a bug (not to mention a security/weakness)?
> > > > > 
> > > > > Looks like the patch fixes a warning which can crash some kernels.  So
> > > > > the CVE appears to be fixing that, rather than the impossible deadlock.
> > > > 
> > > > Are you talking about lockdep warning or anything else?
> > > 
> > > Anything that triggers a BUG() or a WARN() (as per the splat in the
> > > commit message).  Many in-field kernels are configured to panic on
> > > BUG()s and WARN()s, thus triggering them are presently considered local
> > > DoS and attract CVE status.
> > 
> > So... because it is possible to configure machine to reboot on
> > warning, now every warning is a security issue?
> > 
> > Lockdep is for debugging, if someone uses it in production with panic
> > on reboot, they are getting exactly what they are asking for.
> > 
> > Not a security problem.
> 
> And we agree, I don't know what you are arguing about here, please stop.

So you agree that WARN triggering randomly is not a security problem?

Following communication did not say so.

"The splat in the circular lockdep detection code appears to be generated
using some stacked pr_warn() calls, rather than a WARN()."
									Pavel
-- 
People of Russia, stop Putin before his war on Ukraine escalates.

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

Re: CVE-2024-26628: drm/amdkfd: Fix lock dependency warning

[Greg Kroah-Hartman]

On Thu, Jun 13, 2024 at 12:40:35PM +0200, Pavel Machek wrote:
> On Thu 2024-06-13 12:16:50, Greg Kroah-Hartman wrote:
> > On Thu, Jun 13, 2024 at 11:32:41AM +0200, Pavel Machek wrote:
> > > On Wed 2024-03-20 15:47:34, Lee Jones wrote:
> > > > On Wed, 20 Mar 2024, Michal Hocko wrote:
> > > > 
> > > > > On Thu 14-03-24 11:09:38, Lee Jones wrote:
> > > > > > On Fri, 08 Mar 2024, Michal Hocko wrote:
> > > > > > 
> > > > > > > On Wed 06-03-24 06:46:11, Greg KH wrote:
> > > > > > > [...]
> > > > > > > >  Possible unsafe locking scenario:
> > > > > > > > 
> > > > > > > >        CPU0                    CPU1
> > > > > > > >        ----                    ----
> > > > > > > >   lock(&svms->lock);
> > > > > > > >                                lock(&mm->mmap_lock);
> > > > > > > >                                lock(&svms->lock);
> > > > > > > >   lock((work_completion)(&svm_bo->eviction_work));
> > > > > > > > 
> > > > > > > > I believe this cannot really lead to a deadlock in practice, because
> > > > > > > > svm_range_evict_svm_bo_worker only takes the mmap_read_lock if the BO
> > > > > > > > refcount is non-0. That means it's impossible that svm_range_bo_release
> > > > > > > > is running concurrently. However, there is no good way to annotate this.
> > > > > > > 
> > > > > > > OK, so is this even a bug (not to mention a security/weakness)?
> > > > > > 
> > > > > > Looks like the patch fixes a warning which can crash some kernels.  So
> > > > > > the CVE appears to be fixing that, rather than the impossible deadlock.
> > > > > 
> > > > > Are you talking about lockdep warning or anything else?
> > > > 
> > > > Anything that triggers a BUG() or a WARN() (as per the splat in the
> > > > commit message).  Many in-field kernels are configured to panic on
> > > > BUG()s and WARN()s, thus triggering them are presently considered local
> > > > DoS and attract CVE status.
> > > 
> > > So... because it is possible to configure machine to reboot on
> > > warning, now every warning is a security issue?
> > > 
> > > Lockdep is for debugging, if someone uses it in production with panic
> > > on reboot, they are getting exactly what they are asking for.
> > > 
> > > Not a security problem.
> > 
> > And we agree, I don't know what you are arguing about here, please stop.
> 
> So you agree that WARN triggering randomly is not a security problem?
> 
> Following communication did not say so.
> 
> "The splat in the circular lockdep detection code appears to be generated
> using some stacked pr_warn() calls, rather than a WARN()."

*plonk*

Re: CVE-2024-26628: drm/amdkfd: Fix lock dependency warning

[Lee Jones]

On Thu, 13 Jun 2024, Pavel Machek wrote:

> On Thu 2024-06-13 12:16:50, Greg Kroah-Hartman wrote:
> > On Thu, Jun 13, 2024 at 11:32:41AM +0200, Pavel Machek wrote:
> > > On Wed 2024-03-20 15:47:34, Lee Jones wrote:
> > > > On Wed, 20 Mar 2024, Michal Hocko wrote:
> > > > 
> > > > > On Thu 14-03-24 11:09:38, Lee Jones wrote:
> > > > > > On Fri, 08 Mar 2024, Michal Hocko wrote:
> > > > > > 
> > > > > > > On Wed 06-03-24 06:46:11, Greg KH wrote:
> > > > > > > [...]
> > > > > > > >  Possible unsafe locking scenario:
> > > > > > > > 
> > > > > > > >        CPU0                    CPU1
> > > > > > > >        ----                    ----
> > > > > > > >   lock(&svms->lock);
> > > > > > > >                                lock(&mm->mmap_lock);
> > > > > > > >                                lock(&svms->lock);
> > > > > > > >   lock((work_completion)(&svm_bo->eviction_work));
> > > > > > > > 
> > > > > > > > I believe this cannot really lead to a deadlock in practice, because
> > > > > > > > svm_range_evict_svm_bo_worker only takes the mmap_read_lock if the BO
> > > > > > > > refcount is non-0. That means it's impossible that svm_range_bo_release
> > > > > > > > is running concurrently. However, there is no good way to annotate this.
> > > > > > > 
> > > > > > > OK, so is this even a bug (not to mention a security/weakness)?
> > > > > > 
> > > > > > Looks like the patch fixes a warning which can crash some kernels.  So
> > > > > > the CVE appears to be fixing that, rather than the impossible deadlock.
> > > > > 
> > > > > Are you talking about lockdep warning or anything else?
> > > > 
> > > > Anything that triggers a BUG() or a WARN() (as per the splat in the
> > > > commit message).  Many in-field kernels are configured to panic on
> > > > BUG()s and WARN()s, thus triggering them are presently considered local
> > > > DoS and attract CVE status.
> > > 
> > > So... because it is possible to configure machine to reboot on
> > > warning, now every warning is a security issue?
> > > 
> > > Lockdep is for debugging, if someone uses it in production with panic
> > > on reboot, they are getting exactly what they are asking for.
> > > 
> > > Not a security problem.
> > 
> > And we agree, I don't know what you are arguing about here, please stop.
> 
> So you agree that WARN triggering randomly is not a security problem?
> 
> Following communication did not say so.
> 
> "The splat in the circular lockdep detection code appears to be generated
> using some stacked pr_warn() calls, rather than a WARN()."

We agree that the lockdep detection is a debugging feature AND that even
though the splat looks like a WARN(), it does not behave like one.
Therefore it does not constitute a security issue.

However, yes, we believe that if an attacker can trip a WARN() and
reboot a victim's machine on demand then this is equivalent to a local
DoS attack and merits CVE status.

-- 
Lee Jones [李琼斯]