mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
* Re: [PATCH net] dst: fix races in rt6_uncached_list_del() and rt_del_uncached_list()
       [not found] <20260112103825.3810713-1-edumazet@google.com>
@ 2026-02-24 11:41 ` Hillf Danton
  2026-02-24 11:58   ` Eric Dumazet
  0 siblings, 1 reply; 4+ messages in thread
From: Hillf Danton @ 2026-02-24 11:41 UTC (permalink / raw)
  To: Eric Dumazet
  Cc: linux-kernel, netdev, syzbot+179fc225724092b8b2b2, Martin KaFai Lau

On Mon, 12 Jan 2026 10:38:25 +0000 Eric Dumazet wrote:
> syzbot was able to crash the kernel in rt6_uncached_list_flush_dev()
> in an interesting way [1]
> 
> Crash happens in list_del_init()/INIT_LIST_HEAD() while writing
> list->prev, while the prior write on list->next went well.
> 
> static inline void INIT_LIST_HEAD(struct list_head *list)
> {
> 	WRITE_ONCE(list->next, list); // This went well
> 	WRITE_ONCE(list->prev, list); // Crash, @list has been freed.
> }
> 
> Issue here is that rt6_uncached_list_del() did not attempt to lock
> ul->lock, as list_empty(&rt->dst.rt_uncached) returned
> true because the WRITE_ONCE(list->next, list) happened on the other CPU.
> 
> We might use list_del_init_careful() and list_empty_careful(),
> or make sure rt6_uncached_list_del() always grabs the spinlock
> whenever rt->dst.rt_uncached_list has been set.
> 
> A similar fix is neeed for IPv4.
> 
FYI list corruption in dst_destroy() [1] happened after this fix.


[1] Subject: Re: [syzbot] [kernel?] INFO: task hung in restrict_one_thread_callback
https://lore.kernel.org/lkml/699d155f.050a0220.247d23.0377.GAE@google.com/

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH net] dst: fix races in rt6_uncached_list_del() and rt_del_uncached_list()
  2026-02-24 11:41 ` [PATCH net] dst: fix races in rt6_uncached_list_del() and rt_del_uncached_list() Hillf Danton
@ 2026-02-24 11:58   ` Eric Dumazet
  2026-02-25  7:33     ` Christian A. Ehrhardt
  0 siblings, 1 reply; 4+ messages in thread
From: Eric Dumazet @ 2026-02-24 11:58 UTC (permalink / raw)
  To: Hillf Danton
  Cc: linux-kernel, netdev, syzbot+179fc225724092b8b2b2, Martin KaFai Lau

On Tue, Feb 24, 2026 at 12:41 PM Hillf Danton <hdanton@sina.com> wrote:
>
> On Mon, 12 Jan 2026 10:38:25 +0000 Eric Dumazet wrote:
> > syzbot was able to crash the kernel in rt6_uncached_list_flush_dev()
> > in an interesting way [1]
> >
> > Crash happens in list_del_init()/INIT_LIST_HEAD() while writing
> > list->prev, while the prior write on list->next went well.
> >
> > static inline void INIT_LIST_HEAD(struct list_head *list)
> > {
> >       WRITE_ONCE(list->next, list); // This went well
> >       WRITE_ONCE(list->prev, list); // Crash, @list has been freed.
> > }
> >
> > Issue here is that rt6_uncached_list_del() did not attempt to lock
> > ul->lock, as list_empty(&rt->dst.rt_uncached) returned
> > true because the WRITE_ONCE(list->next, list) happened on the other CPU.
> >
> > We might use list_del_init_careful() and list_empty_careful(),
> > or make sure rt6_uncached_list_del() always grabs the spinlock
> > whenever rt->dst.rt_uncached_list has been set.
> >
> > A similar fix is neeed for IPv4.
> >
> FYI list corruption in dst_destroy() [1] happened after this fix.
>
>
> [1] Subject: Re: [syzbot] [kernel?] INFO: task hung in restrict_one_thread_callback
> https://lore.kernel.org/lkml/699d155f.050a0220.247d23.0377.GAE@google.com/

Seems unrelated.

Are you seeing anything wrong with my patch ?

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH net] dst: fix races in rt6_uncached_list_del() and rt_del_uncached_list()
  2026-02-24 11:58   ` Eric Dumazet
@ 2026-02-25  7:33     ` Christian A. Ehrhardt
  2026-02-25  8:15       ` Eric Dumazet
  0 siblings, 1 reply; 4+ messages in thread
From: Christian A. Ehrhardt @ 2026-02-25  7:33 UTC (permalink / raw)
  To: Eric Dumazet
  Cc: Hillf Danton, linux-kernel, netdev, syzbot+179fc225724092b8b2b2,
	Martin KaFai Lau


Hi,

On Tue, Feb 24, 2026 at 12:58:25PM +0100, Eric Dumazet wrote:
> On Tue, Feb 24, 2026 at 12:41 PM Hillf Danton <hdanton@sina.com> wrote:
> >
> > On Mon, 12 Jan 2026 10:38:25 +0000 Eric Dumazet wrote:
> > > syzbot was able to crash the kernel in rt6_uncached_list_flush_dev()
> > > in an interesting way [1]
> > >
> > > Crash happens in list_del_init()/INIT_LIST_HEAD() while writing
> > > list->prev, while the prior write on list->next went well.
> > >
> > > static inline void INIT_LIST_HEAD(struct list_head *list)
> > > {
> > >       WRITE_ONCE(list->next, list); // This went well
> > >       WRITE_ONCE(list->prev, list); // Crash, @list has been freed.
> > > }
> > >
> > > Issue here is that rt6_uncached_list_del() did not attempt to lock
> > > ul->lock, as list_empty(&rt->dst.rt_uncached) returned
> > > true because the WRITE_ONCE(list->next, list) happened on the other CPU.
> > >
> > > We might use list_del_init_careful() and list_empty_careful(),
> > > or make sure rt6_uncached_list_del() always grabs the spinlock
> > > whenever rt->dst.rt_uncached_list has been set.
> > >
> > > A similar fix is neeed for IPv4.
> > >
> > FYI list corruption in dst_destroy() [1] happened after this fix.
> >
> >
> > [1] Subject: Re: [syzbot] [kernel?] INFO: task hung in restrict_one_thread_callback
> > https://lore.kernel.org/lkml/699d155f.050a0220.247d23.0377.GAE@google.com/
> 
> Seems unrelated.
> 
> Are you seeing anything wrong with my patch ?

I'm certainly not an expert on this code and I might well be wrong
but it seems that with your patch your are racing against a list
delete in rt6_uncached_list_flush_dev which removes the item from
the list but does not reset dst.rt_uncached_list.

So I think you want to keep the list_empty() check but do it
under the lock?

Best regards,
Christian


^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH net] dst: fix races in rt6_uncached_list_del() and rt_del_uncached_list()
  2026-02-25  7:33     ` Christian A. Ehrhardt
@ 2026-02-25  8:15       ` Eric Dumazet
  0 siblings, 0 replies; 4+ messages in thread
From: Eric Dumazet @ 2026-02-25  8:15 UTC (permalink / raw)
  To: Christian A. Ehrhardt
  Cc: Hillf Danton, linux-kernel, netdev, syzbot+179fc225724092b8b2b2,
	Martin KaFai Lau

On Wed, Feb 25, 2026 at 8:33 AM Christian A. Ehrhardt <lk@c--e.de> wrote:
>
>
> Hi,
>
> On Tue, Feb 24, 2026 at 12:58:25PM +0100, Eric Dumazet wrote:
> > On Tue, Feb 24, 2026 at 12:41 PM Hillf Danton <hdanton@sina.com> wrote:
> > >
> > > On Mon, 12 Jan 2026 10:38:25 +0000 Eric Dumazet wrote:
> > > > syzbot was able to crash the kernel in rt6_uncached_list_flush_dev()
> > > > in an interesting way [1]
> > > >
> > > > Crash happens in list_del_init()/INIT_LIST_HEAD() while writing
> > > > list->prev, while the prior write on list->next went well.
> > > >
> > > > static inline void INIT_LIST_HEAD(struct list_head *list)
> > > > {
> > > >       WRITE_ONCE(list->next, list); // This went well
> > > >       WRITE_ONCE(list->prev, list); // Crash, @list has been freed.
> > > > }
> > > >
> > > > Issue here is that rt6_uncached_list_del() did not attempt to lock
> > > > ul->lock, as list_empty(&rt->dst.rt_uncached) returned
> > > > true because the WRITE_ONCE(list->next, list) happened on the other CPU.
> > > >
> > > > We might use list_del_init_careful() and list_empty_careful(),
> > > > or make sure rt6_uncached_list_del() always grabs the spinlock
> > > > whenever rt->dst.rt_uncached_list has been set.
> > > >
> > > > A similar fix is neeed for IPv4.
> > > >
> > > FYI list corruption in dst_destroy() [1] happened after this fix.
> > >
> > >
> > > [1] Subject: Re: [syzbot] [kernel?] INFO: task hung in restrict_one_thread_callback
> > > https://lore.kernel.org/lkml/699d155f.050a0220.247d23.0377.GAE@google.com/
> >
> > Seems unrelated.
> >
> > Are you seeing anything wrong with my patch ?
>
> I'm certainly not an expert on this code and I might well be wrong
> but it seems that with your patch your are racing against a list
> delete in rt6_uncached_list_flush_dev which removes the item from
> the list but does not reset dst.rt_uncached_list.

Both rt_del_uncached_list() and rt6_uncached_list_flush_dev() use
list_del_init()
with the spinlock held. No race is possible.

>
> So I think you want to keep the list_empty() check but do it
> under the lock?

A second list_del_init() is a NOP, so adding a list_empty() would only
add extra code
for a condition which is already handled.

>
> Best regards,
> Christian

Thanks.

^ permalink raw reply	[flat|nested] 4+ messages in thread

end of thread, other threads:[~2026-02-25  8:15 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
     [not found] <20260112103825.3810713-1-edumazet@google.com>
2026-02-24 11:41 ` [PATCH net] dst: fix races in rt6_uncached_list_del() and rt_del_uncached_list() Hillf Danton
2026-02-24 11:58   ` Eric Dumazet
2026-02-25  7:33     ` Christian A. Ehrhardt
2026-02-25  8:15       ` Eric Dumazet

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox

all inboxes | Powered by JetHome®