[PATCH net] can: isotp: take rtnl_lock() before leaving the notifier list

[PATCH net] can: isotp: take rtnl_lock() before leaving the notifier list

[Norbert Szetei]

isotp_release() removes the socket from isotp_notifier_list before it
takes rtnl_lock(). The netdev notifier chain runs under RTNL, so a
socket that leaves the list in that window is skipped by isotp_notify()
and has to unregister its own CAN filters.

It cannot always do that. isotp_release() passes sock_net(sk) to
can_rx_unregister(), which returns early when that netns no longer
matches dev_net(dev), before the receiver list is searched and before
the "receive list entry not found" warning. Once the bound device has
been moved to another netns the filters are removed zero times, and
can_rx_register() stores rcv->sk without taking a reference, so the
receivers left in the device's dev_rcv_lists point at the freed socket
and travel with the device into the new netns.

  BUG: KASAN: use-after-free in isotp_rcv+0x1570/0x24d0
  Read of size 1 at addr ffff888118130552 by task isotp_ns_uaf/578
   can_rcv_filter+0x4af/0x8c0
   can_receive+0x28d/0x3c0
   can_rcv+0x2a9/0x310
   __netif_receive_skb_one_core+0x21a/0x260
   process_backlog+0x210/0x760

Take rtnl_lock() before removing the socket from the notifier list, so
that isotp_release() and isotp_notify() cannot both skip the removal.

Fixes: 20bab8b88baa ("can: isotp: fix use-after-free race with concurrent NETDEV_UNREGISTER")
Cc: stable@vger.kernel.org
Assisted-by: Claude:claude-opus-5
Signed-off-by: Norbert Szetei <norbert@doyensec.com>
---
Reproducer available on request.

 net/can/isotp.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/net/can/isotp.c b/net/can/isotp.c
index 155530aedce2..8ca75d30360c 100644
--- a/net/can/isotp.c
+++ b/net/can/isotp.c
@@ -1475,6 +1475,8 @@ static int isotp_release(struct socket *sock)
 	/* forced SHUTDOWN may have skipped IDLE (gave up on a signal) */
 	wake_up_interruptible(&so->wait);
 
+	rtnl_lock();
+
 	spin_lock(&isotp_notifier_lock);
 	while (isotp_busy_notifier == so) {
 		spin_unlock(&isotp_notifier_lock);
@@ -1484,7 +1486,6 @@ static int isotp_release(struct socket *sock)
 	list_del(&so->notifier);
 	spin_unlock(&isotp_notifier_lock);
 
-	rtnl_lock();
 	lock_sock(sk);
 
 	/* remove current filters & unregister
-- 
2.55.0

Re: [PATCH net] can: isotp: take rtnl_lock() before leaving the notifier list

[Oliver Hartkopp]

Hello Norbert,

many thanks for your patch and the analysis of the unremoved filter 
lists in the case of moving a CAN device to another namespace.

But I don't think that moving rtnl_lock() up so that it covers a busy 
loop including a schedule_timeout_uninterruptible(1) wait is not a nice 
move for other rtnl_lock() users.

Focussing on the removal of the correct filter lists when the namespace 
is changed away from the socket's namespace I would propose this small 
change:

diff --git a/net/can/isotp.c b/net/can/isotp.c
index 155530aedce2..0835a4758a72 100644
--- a/net/can/isotp.c
+++ b/net/can/isotp.c
@@ -1490,15 +1490,15 @@ static int isotp_release(struct socket *sock)
         /* remove current filters & unregister
          * tracked reference so->dev is taken at bind() time with rtnl_lock
          */
         if (so->bound && so->dev) {
                 if (isotp_register_rxid(so))
-                       can_rx_unregister(net, so->dev, so->rxid,
+                       can_rx_unregister(dev_net(so->dev), so->dev, 
so->rxid,
                                           SINGLE_MASK(so->rxid),
                                           isotp_rcv, sk);

-               can_rx_unregister(net, so->dev, so->txid,
+               can_rx_unregister(dev_net(so->dev), so->dev, so->txid,
                                   SINGLE_MASK(so->txid),
                                   isotp_rcv_echo, sk);
                 netdev_put(so->dev, &so->dev_tracker);
         }

@@ -1846,13 +1846,10 @@ static int isotp_getsockopt(struct socket *sock, 
int level, int optname,
  static void isotp_notify(struct isotp_sock *so, unsigned long msg,
                          struct net_device *dev)
  {
         struct sock *sk = &so->sk;

-       if (!net_eq(dev_net(dev), sock_net(sk)))
-               return;
-
         if (so->dev != dev)
                 return;

         switch (msg) {
         case NETDEV_UNREGISTER:


Can you give it a try with your KASAN setup and maybe also ask opus 
about my idea?

Many thanks,
Oliver

On 31.08.26 10:30, Norbert Szetei wrote:
> isotp_release() removes the socket from isotp_notifier_list before it
> takes rtnl_lock(). The netdev notifier chain runs under RTNL, so a
> socket that leaves the list in that window is skipped by isotp_notify()
> and has to unregister its own CAN filters.
> 
> It cannot always do that. isotp_release() passes sock_net(sk) to
> can_rx_unregister(), which returns early when that netns no longer
> matches dev_net(dev), before the receiver list is searched and before
> the "receive list entry not found" warning. Once the bound device has
> been moved to another netns the filters are removed zero times, and
> can_rx_register() stores rcv->sk without taking a reference, so the
> receivers left in the device's dev_rcv_lists point at the freed socket
> and travel with the device into the new netns.
> 
>    BUG: KASAN: use-after-free in isotp_rcv+0x1570/0x24d0
>    Read of size 1 at addr ffff888118130552 by task isotp_ns_uaf/578
>     can_rcv_filter+0x4af/0x8c0
>     can_receive+0x28d/0x3c0
>     can_rcv+0x2a9/0x310
>     __netif_receive_skb_one_core+0x21a/0x260
>     process_backlog+0x210/0x760
> 
> Take rtnl_lock() before removing the socket from the notifier list, so
> that isotp_release() and isotp_notify() cannot both skip the removal.
> 
> Fixes: 20bab8b88baa ("can: isotp: fix use-after-free race with concurrent NETDEV_UNREGISTER")
> Cc: stable@vger.kernel.org
> Assisted-by: Claude:claude-opus-5
> Signed-off-by: Norbert Szetei <norbert@doyensec.com>
> ---
> Reproducer available on request.
> 
>   net/can/isotp.c | 3 ++-
>   1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/net/can/isotp.c b/net/can/isotp.c
> index 155530aedce2..8ca75d30360c 100644
> --- a/net/can/isotp.c
> +++ b/net/can/isotp.c
> @@ -1475,6 +1475,8 @@ static int isotp_release(struct socket *sock)
>   	/* forced SHUTDOWN may have skipped IDLE (gave up on a signal) */
>   	wake_up_interruptible(&so->wait);
>   
> +	rtnl_lock();
> +
>   	spin_lock(&isotp_notifier_lock);
>   	while (isotp_busy_notifier == so) {
>   		spin_unlock(&isotp_notifier_lock);
> @@ -1484,7 +1486,6 @@ static int isotp_release(struct socket *sock)
>   	list_del(&so->notifier);
>   	spin_unlock(&isotp_notifier_lock);
>   
> -	rtnl_lock();
>   	lock_sock(sk);
>   
>   	/* remove current filters & unregister

Re: [PATCH net] can: isotp: take rtnl_lock() before leaving the notifier list

[Norbert Szetei]

Hey Oliver,

> On Aug 31, 2026, at 15:08, Oliver Hartkopp <socketcan@hartkopp.net> wrote:
> 
> Hello Norbert,
> 
> many thanks for your patch and the analysis of the unremoved filter lists in the case of moving a CAN device to another namespace.
> 
> But I don't think that moving rtnl_lock() up so that it covers a busy loop including a schedule_timeout_uninterruptible(1) wait is not a nice move for other rtnl_lock() users.
> 
> Focussing on the removal of the correct filter lists when the namespace is changed away from the socket's namespace I would propose this small change:
> 
> diff --git a/net/can/isotp.c b/net/can/isotp.c
> index 155530aedce2..0835a4758a72 100644
> --- a/net/can/isotp.c
> +++ b/net/can/isotp.c
> @@ -1490,15 +1490,15 @@ static int isotp_release(struct socket *sock)
>        /* remove current filters & unregister
>         * tracked reference so->dev is taken at bind() time with rtnl_lock
>         */
>        if (so->bound && so->dev) {
>                if (isotp_register_rxid(so))
> -                       can_rx_unregister(net, so->dev, so->rxid,
> +                       can_rx_unregister(dev_net(so->dev), so->dev, so->rxid,
>                                          SINGLE_MASK(so->rxid),
>                                          isotp_rcv, sk);
> 
> -               can_rx_unregister(net, so->dev, so->txid,
> +               can_rx_unregister(dev_net(so->dev), so->dev, so->txid,
>                                  SINGLE_MASK(so->txid),
>                                  isotp_rcv_echo, sk);
>                netdev_put(so->dev, &so->dev_tracker);
>        }
> 
> @@ -1846,13 +1846,10 @@ static int isotp_getsockopt(struct socket *sock, int level, int optname,
> static void isotp_notify(struct isotp_sock *so, unsigned long msg,
>                         struct net_device *dev)
> {
>        struct sock *sk = &so->sk;
> 
> -       if (!net_eq(dev_net(dev), sock_net(sk)))
> -               return;
> -
>        if (so->dev != dev)
>                return;
> 
>        switch (msg) {
>        case NETDEV_UNREGISTER:
> 
> 
> Can you give it a try with your KASAN setup and maybe also ask opus about my idea?

I just tested your version and I was no longer able to reproduce 
the bug. Initially, I considered it too, but moving rtnl_lock() 
sounded simpler and I had not thought about the busy-wait sitting 
there. Thanks for pointing this out and submitting the patch.

Regards,
Norbert

> Many thanks,
> Oliver
> 
> On 31.08.26 10:30, Norbert Szetei wrote:
>> isotp_release() removes the socket from isotp_notifier_list before it
>> takes rtnl_lock(). The netdev notifier chain runs under RTNL, so a
>> socket that leaves the list in that window is skipped by isotp_notify()
>> and has to unregister its own CAN filters.
>> It cannot always do that. isotp_release() passes sock_net(sk) to
>> can_rx_unregister(), which returns early when that netns no longer
>> matches dev_net(dev), before the receiver list is searched and before
>> the "receive list entry not found" warning. Once the bound device has
>> been moved to another netns the filters are removed zero times, and
>> can_rx_register() stores rcv->sk without taking a reference, so the
>> receivers left in the device's dev_rcv_lists point at the freed socket
>> and travel with the device into the new netns.
>>   BUG: KASAN: use-after-free in isotp_rcv+0x1570/0x24d0
>>   Read of size 1 at addr ffff888118130552 by task isotp_ns_uaf/578
>>    can_rcv_filter+0x4af/0x8c0
>>    can_receive+0x28d/0x3c0
>>    can_rcv+0x2a9/0x310
>>    __netif_receive_skb_one_core+0x21a/0x260
>>    process_backlog+0x210/0x760
>> Take rtnl_lock() before removing the socket from the notifier list, so
>> that isotp_release() and isotp_notify() cannot both skip the removal.
>> Fixes: 20bab8b88baa ("can: isotp: fix use-after-free race with concurrent NETDEV_UNREGISTER")
>> Cc: stable@vger.kernel.org
>> Assisted-by: Claude:claude-opus-5
>> Signed-off-by: Norbert Szetei <norbert@doyensec.com>
>> ---
>> Reproducer available on request.
>>  net/can/isotp.c | 3 ++-
>>  1 file changed, 2 insertions(+), 1 deletion(-)
>> diff --git a/net/can/isotp.c b/net/can/isotp.c
>> index 155530aedce2..8ca75d30360c 100644
>> --- a/net/can/isotp.c
>> +++ b/net/can/isotp.c
>> @@ -1475,6 +1475,8 @@ static int isotp_release(struct socket *sock)
>>   /* forced SHUTDOWN may have skipped IDLE (gave up on a signal) */
>>   wake_up_interruptible(&so->wait);
>>  + rtnl_lock();
>> +
>>   spin_lock(&isotp_notifier_lock);
>>   while (isotp_busy_notifier == so) {
>>   spin_unlock(&isotp_notifier_lock);
>> @@ -1484,7 +1486,6 @@ static int isotp_release(struct socket *sock)
>>   list_del(&so->notifier);
>>   spin_unlock(&isotp_notifier_lock);
>>  - rtnl_lock();
>>   lock_sock(sk);
>>     /* remove current filters & unregister
> 

Re: [PATCH net] can: isotp: take rtnl_lock() before leaving the notifier list

[Oliver Hartkopp]

Hi Norbert!

On 01.09.26 09:01, Norbert Szetei wrote:
> Hey Oliver,
> 
>> On Aug 31, 2026, at 15:08, Oliver Hartkopp <socketcan@hartkopp.net> wrote:
>>
>> Hello Norbert,
>>
>> many thanks for your patch and the analysis of the unremoved filter lists in the case of moving a CAN device to another namespace.
>>
>> But I don't think that moving rtnl_lock() up so that it covers a busy loop including a schedule_timeout_uninterruptible(1) wait is not a nice move for other rtnl_lock() users.
>>
>> Focussing on the removal of the correct filter lists when the namespace is changed away from the socket's namespace I would propose this small change:
>>
>> diff --git a/net/can/isotp.c b/net/can/isotp.c
>> index 155530aedce2..0835a4758a72 100644
>> --- a/net/can/isotp.c
>> +++ b/net/can/isotp.c
>> @@ -1490,15 +1490,15 @@ static int isotp_release(struct socket *sock)
>>         /* remove current filters & unregister
>>          * tracked reference so->dev is taken at bind() time with rtnl_lock
>>          */
>>         if (so->bound && so->dev) {
>>                 if (isotp_register_rxid(so))
>> -                       can_rx_unregister(net, so->dev, so->rxid,
>> +                       can_rx_unregister(dev_net(so->dev), so->dev, so->rxid,
>>                                           SINGLE_MASK(so->rxid),
>>                                           isotp_rcv, sk);
>>
>> -               can_rx_unregister(net, so->dev, so->txid,
>> +               can_rx_unregister(dev_net(so->dev), so->dev, so->txid,
>>                                   SINGLE_MASK(so->txid),
>>                                   isotp_rcv_echo, sk);
>>                 netdev_put(so->dev, &so->dev_tracker);
>>         }
>>
>> @@ -1846,13 +1846,10 @@ static int isotp_getsockopt(struct socket *sock, int level, int optname,
>> static void isotp_notify(struct isotp_sock *so, unsigned long msg,
>>                          struct net_device *dev)
>> {
>>         struct sock *sk = &so->sk;
>>
>> -       if (!net_eq(dev_net(dev), sock_net(sk)))
>> -               return;
>> -
>>         if (so->dev != dev)
>>                 return;
>>
>>         switch (msg) {
>>         case NETDEV_UNREGISTER:
>>
>>
>> Can you give it a try with your KASAN setup and maybe also ask opus about my idea?
> 
> I just tested your version and I was no longer able to reproduce
> the bug. Initially, I considered it too, but moving rtnl_lock()
> sounded simpler and I had not thought about the busy-wait sitting
> there. Thanks for pointing this out and submitting the patch.

Thanks for testing!

Btw. sashiko bot pointed out some inconvenience with the removed 
net_eq() check, as I'm checking for ifindex equality in bcm.c at some 
places - and the ifindex values are not unique over all namespaces like 
the struct netdev *dev pointer.

https://lore.kernel.org/linux-can/20260831212432.6C2B51F000E9@smtp.kernel.org/

So I need to extend bcm.c in a way that it is checking the dev pointers 
instead of dev->ifindex in those places. There will be a v2 soon.

Btw. many thanks for testing that the original root cause was fixed with 
this approach.

Best regards,
Oliver


> 
> Regards,
> Norbert
> 
>> Many thanks,
>> Oliver
>>
>> On 31.08.26 10:30, Norbert Szetei wrote:
>>> isotp_release() removes the socket from isotp_notifier_list before it
>>> takes rtnl_lock(). The netdev notifier chain runs under RTNL, so a
>>> socket that leaves the list in that window is skipped by isotp_notify()
>>> and has to unregister its own CAN filters.
>>> It cannot always do that. isotp_release() passes sock_net(sk) to
>>> can_rx_unregister(), which returns early when that netns no longer
>>> matches dev_net(dev), before the receiver list is searched and before
>>> the "receive list entry not found" warning. Once the bound device has
>>> been moved to another netns the filters are removed zero times, and
>>> can_rx_register() stores rcv->sk without taking a reference, so the
>>> receivers left in the device's dev_rcv_lists point at the freed socket
>>> and travel with the device into the new netns.
>>>    BUG: KASAN: use-after-free in isotp_rcv+0x1570/0x24d0
>>>    Read of size 1 at addr ffff888118130552 by task isotp_ns_uaf/578
>>>     can_rcv_filter+0x4af/0x8c0
>>>     can_receive+0x28d/0x3c0
>>>     can_rcv+0x2a9/0x310
>>>     __netif_receive_skb_one_core+0x21a/0x260
>>>     process_backlog+0x210/0x760
>>> Take rtnl_lock() before removing the socket from the notifier list, so
>>> that isotp_release() and isotp_notify() cannot both skip the removal.
>>> Fixes: 20bab8b88baa ("can: isotp: fix use-after-free race with concurrent NETDEV_UNREGISTER")
>>> Cc: stable@vger.kernel.org
>>> Assisted-by: Claude:claude-opus-5
>>> Signed-off-by: Norbert Szetei <norbert@doyensec.com>
>>> ---
>>> Reproducer available on request.
>>>   net/can/isotp.c | 3 ++-
>>>   1 file changed, 2 insertions(+), 1 deletion(-)
>>> diff --git a/net/can/isotp.c b/net/can/isotp.c
>>> index 155530aedce2..8ca75d30360c 100644
>>> --- a/net/can/isotp.c
>>> +++ b/net/can/isotp.c
>>> @@ -1475,6 +1475,8 @@ static int isotp_release(struct socket *sock)
>>>    /* forced SHUTDOWN may have skipped IDLE (gave up on a signal) */
>>>    wake_up_interruptible(&so->wait);
>>>   + rtnl_lock();
>>> +
>>>    spin_lock(&isotp_notifier_lock);
>>>    while (isotp_busy_notifier == so) {
>>>    spin_unlock(&isotp_notifier_lock);
>>> @@ -1484,7 +1486,6 @@ static int isotp_release(struct socket *sock)
>>>    list_del(&so->notifier);
>>>    spin_unlock(&isotp_notifier_lock);
>>>   - rtnl_lock();
>>>    lock_sock(sk);
>>>      /* remove current filters & unregister
>>
>