[Bug] "possible deadlock in rtnl_newlink" in Linux kernel v6.13

[Bug] "possible deadlock in rtnl_newlink" in Linux kernel v6.13

[John]

Dear Linux Kernel Maintainers,

I hope this message finds you well.

I am writing to report a potential vulnerability I encountered during
testing of the Linux Kernel version v6.13.

Git Commit: ffd294d346d185b70e28b1a28abe367bbfe53c04 (tag: v6.13)

Bug Location: rtnl_newlink+0x86c/0x1dd0 net/core/rtnetlink.c:4011

Bug report: https://hastebin.com/share/ajavibofik.bash

Complete log: https://hastebin.com/share/derufumuxu.perl

Entire kernel config:  https://hastebin.com/share/lovayaqidu.ini

Root Cause Analysis:
The deadlock warning is caused by a circular locking dependency
between two subsystems:

Path A (CPU 0):
Holds rtnl_mutex in rtnl_newlink() →
Then calls e1000_close() →
Triggers e1000_down_and_stop() →
Calls __cancel_work_sync() →
Tries to flush adapter->reset_task (→ needs work_completion lock)

Path B (CPU 1):
Holds work_completion lock while running e1000_reset_task() →
Then calls e1000_down() →
Which tries to acquire rtnl_mutex
These two execution paths result in a circular dependency:

CPU 0: rtnl_mutex → work_completion
CPU 1: work_completion → rtnl_mutex

This violates lock ordering and can lead to a deadlock under contention.
This bug represents a classic case of lock inversion between
networking core (rtnl_mutex) and a device driver (e1000 workqueue
reset`).
It is a design-level concurrency flaw that can lead to deadlocks under
stress or fuzzing workloads.

At present, I have not yet obtained a minimal reproducer for this
issue. However, I am actively working on reproducing it, and I will
promptly share any additional findings or a working reproducer as soon
as it becomes available.

Thank you very much for your time and attention to this matter. I
truly appreciate the efforts of the Linux kernel community.

Best regards,
John

Re: [Bug] "possible deadlock in rtnl_newlink" in Linux kernel v6.13

[Jacob Keller]

On 5/21/2025 5:52 PM, John wrote:
> Dear Linux Kernel Maintainers,
> 
> I hope this message finds you well.
> 
> I am writing to report a potential vulnerability I encountered during
> testing of the Linux Kernel version v6.13.
> 
> Git Commit: ffd294d346d185b70e28b1a28abe367bbfe53c04 (tag: v6.13)
> 
> Bug Location: rtnl_newlink+0x86c/0x1dd0 net/core/rtnetlink.c:4011
> 
> Bug report: https://hastebin.com/share/ajavibofik.bash
> 
> Complete log: https://hastebin.com/share/derufumuxu.perl
> 
> Entire kernel config:  https://hastebin.com/share/lovayaqidu.ini
> 
> Root Cause Analysis:
> The deadlock warning is caused by a circular locking dependency
> between two subsystems:
> 
> Path A (CPU 0):
> Holds rtnl_mutex in rtnl_newlink() →
> Then calls e1000_close() →
> Triggers e1000_down_and_stop() →
> Calls __cancel_work_sync() →
> Tries to flush adapter->reset_task (→ needs work_completion lock)
> 
> Path B (CPU 1):
> Holds work_completion lock while running e1000_reset_task() →
> Then calls e1000_down() →
> Which tries to acquire rtnl_mutex
> These two execution paths result in a circular dependency:
> 

I guess this implies you can't cancel_work_sync while holding RTNL lock?
Hmm. Or maybe its because calling e1000_down from the e1000_reset_task
is a problem.

> CPU 0: rtnl_mutex → work_completion
> CPU 1: work_completion → rtnl_mutex
> 
> This violates lock ordering and can lead to a deadlock under contention.
> This bug represents a classic case of lock inversion between
> networking core (rtnl_mutex) and a device driver (e1000 workqueue
> reset`).
> It is a design-level concurrency flaw that can lead to deadlocks under
> stress or fuzzing workloads.
> 
> At present, I have not yet obtained a minimal reproducer for this
> issue. However, I am actively working on reproducing it, and I will
> promptly share any additional findings or a working reproducer as soon
> as it becomes available.
> 

This is likely a regression in e400c7444d84 ("e1000: Hold RTNL when
e1000_down can be called")

@Joe, thoughts?


> Thank you very much for your time and attention to this matter. I
> truly appreciate the efforts of the Linux kernel community.
> 
> Best regards,
> John
> 

Re: [Bug] "possible deadlock in rtnl_newlink" in Linux kernel v6.13

[Joe Damato]

On Thu, May 22, 2025 at 04:05:05PM -0700, Jacob Keller wrote:
> 
> 
> On 5/21/2025 5:52 PM, John wrote:
> > Dear Linux Kernel Maintainers,
> > 
> > I hope this message finds you well.
> > 
> > I am writing to report a potential vulnerability I encountered during
> > testing of the Linux Kernel version v6.13.
> > 
> > Git Commit: ffd294d346d185b70e28b1a28abe367bbfe53c04 (tag: v6.13)
> > 
> > Bug Location: rtnl_newlink+0x86c/0x1dd0 net/core/rtnetlink.c:4011
> > 
> > Bug report: https://hastebin.com/share/ajavibofik.bash
> > 
> > Complete log: https://hastebin.com/share/derufumuxu.perl
> > 
> > Entire kernel config:  https://hastebin.com/share/lovayaqidu.ini
> > 
> > Root Cause Analysis:
> > The deadlock warning is caused by a circular locking dependency
> > between two subsystems:
> > 
> > Path A (CPU 0):
> > Holds rtnl_mutex in rtnl_newlink() ->
> > Then calls e1000_close() ->
> > Triggers e1000_down_and_stop() ->
> > Calls __cancel_work_sync() ->
> > Tries to flush adapter->reset_task (-> needs work_completion lock)
> > 
> > Path B (CPU 1):
> > Holds work_completion lock while running e1000_reset_task() ->
> > Then calls e1000_down() ->
> > Which tries to acquire rtnl_mutex
> > These two execution paths result in a circular dependency:
> > 
> 
> I guess this implies you can't cancel_work_sync while holding RTNL lock?
> Hmm. Or maybe its because calling e1000_down from the e1000_reset_task
> is a problem.
> 
> > CPU 0: rtnl_mutex -> work_completion
> > CPU 1: work_completion -> rtnl_mutex
> > 
> > This violates lock ordering and can lead to a deadlock under contention.
> > This bug represents a classic case of lock inversion between
> > networking core (rtnl_mutex) and a device driver (e1000 workqueue
> > reset`).
> > It is a design-level concurrency flaw that can lead to deadlocks under
> > stress or fuzzing workloads.
> > 
> > At present, I have not yet obtained a minimal reproducer for this
> > issue. However, I am actively working on reproducing it, and I will
> > promptly share any additional findings or a working reproducer as soon
> > as it becomes available.
> > 
> 
> This is likely a regression in e400c7444d84 ("e1000: Hold RTNL when
> e1000_down can be called")
> 
> @Joe, thoughts?

Sorry for the delay, was out of the office for a bit. I agree with
the report that the locking order is problematic and with your
report that it was introduced by the above commit.

I wonder if e1000_down needs to cancel the reset_task at all?

If you look a layer below the original bug report, you'll note that
e1000_down calls e1000_reinit_locked which itself has the following
code:

  /* only run the task if not already down */
  if (!test_bit(__E1000_DOWN, &adapter->flags)) {
          e1000_down(adapter);
          e1000_up(adapter);
  }

So, it seems like the flow in the e1000_down case would be something like this
(please correct me if I've gotten it wrong):

e1000_down -> e1000_down_and_stop (which sets the __E1000_DOWN bit) ->
  cancel_work_sync -> e1000_reset_task -> grabs RTNL, calls e1000_reinit_locked
   e1000_reinit_locked -> checks the bit via the code above and does nothing

I could be totally off here, but it seems like in the e1000_down case, calling
e1000_reinit_locked is unnecessary since the __E1000_DOWN bit prevents anything
from happening.

Maybe a potential solution might be to move the cancel_work_sync out of
e1000_down_and_stop and move it into e1000_remove directly?

Something vaguely like (untested):

diff --git a/drivers/net/ethernet/intel/e1000/e1000_main.c b/drivers/net/ethernet/intel/e1000/e1000_main.c
index 3f089c3d47b2..62a77b34c9ff 100644
--- a/drivers/net/ethernet/intel/e1000/e1000_main.c
+++ b/drivers/net/ethernet/intel/e1000/e1000_main.c
@@ -477,10 +477,6 @@ static void e1000_down_and_stop(struct e1000_adapter *adapter)

        cancel_delayed_work_sync(&adapter->phy_info_task);
        cancel_delayed_work_sync(&adapter->fifo_stall_task);
-
-       /* Only kill reset task if adapter is not resetting */
-       if (!test_bit(__E1000_RESETTING, &adapter->flags))
-               cancel_work_sync(&adapter->reset_task);
 }

 void e1000_down(struct e1000_adapter *adapter)
@@ -1262,6 +1258,11 @@ static void e1000_remove(struct pci_dev *pdev)
        bool disable_dev;

        e1000_down_and_stop(adapter);
+
+       /* Only kill reset task if adapter is not resetting */
+       if (!test_bit(__E1000_RESETTING, &adapter->flags))
+               cancel_work_sync(&adapter->reset_task);
+
        e1000_release_manageability(adapter);

        unregister_netdev(netdev);

Re: [Bug] "possible deadlock in rtnl_newlink" in Linux kernel v6.13

[Jakub Kicinski]

On Thu, 29 May 2025 16:50:17 -0700 Joe Damato wrote:
> @@ -1262,6 +1258,11 @@ static void e1000_remove(struct pci_dev *pdev)
>         bool disable_dev;
> 
>         e1000_down_and_stop(adapter);
> +
> +       /* Only kill reset task if adapter is not resetting */
> +       if (!test_bit(__E1000_RESETTING, &adapter->flags))
> +               cancel_work_sync(&adapter->reset_task);
> +
>         e1000_release_manageability(adapter);
> 
>         unregister_netdev(netdev);

LGTM, FWIW.
For extra points you can move it after the unregister_netdev(),
the existing code cancels the work but netdev may still be up
and kick it back in..

Re: [Bug] "possible deadlock in rtnl_newlink" in Linux kernel v6.13

[Joe Damato]

On Thu, May 29, 2025 at 05:16:40PM -0700, Jakub Kicinski wrote:
> On Thu, 29 May 2025 16:50:17 -0700 Joe Damato wrote:
> > @@ -1262,6 +1258,11 @@ static void e1000_remove(struct pci_dev *pdev)
> >         bool disable_dev;
> > 
> >         e1000_down_and_stop(adapter);
> > +
> > +       /* Only kill reset task if adapter is not resetting */
> > +       if (!test_bit(__E1000_RESETTING, &adapter->flags))
> > +               cancel_work_sync(&adapter->reset_task);
> > +
> >         e1000_release_manageability(adapter);
> > 
> >         unregister_netdev(netdev);
> 
> LGTM, FWIW.
> For extra points you can move it after the unregister_netdev(),
> the existing code cancels the work but netdev may still be up
> and kick it back in..

Good idea, thanks.

I'll post something to the list, but I don't have a reproducer to
test. I'm a noob with syzbot, but maybe there's a way to trigger it
to re-run with a posted patch?

Re: [Bug] "possible deadlock in rtnl_newlink" in Linux kernel v6.13

[Jakub Kicinski]

On Thu, 29 May 2025 18:12:25 -0700 Joe Damato wrote:
> I'll post something to the list, but I don't have a reproducer to
> test. I'm a noob with syzbot, but maybe there's a way to trigger it
> to re-run with a posted patch?

If there is a repro you can post a patch and say #syz test.
You can reply to just syzbot, no need to CC the list.