netdev.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: stranche@codeaurora.org
To: netdev@vger.kernel.org
Cc: subashab@codeaurora.org, dsahern@gmail.com, weiwan@google.com
Subject: List corruption from ipv6_route_seq_start
Date: Thu, 14 Jan 2021 16:47:55 -0700	[thread overview]
Message-ID: <a0d35019167c32728aa810fee91909b5@codeaurora.org> (raw)

Hi everyone,

We've had a list corruption reported to us when using the 
/proc/net/ipv6_route file to read the routing information on the system 
on the 5.4.61 kernel.
 From the list pointers, it seems that the list_head in the fib6_walker 
has been reinitialized with INIT_LIST_HEAD() in 
ipv6_route_seq_setup_walk() while the walker was still on the 
fib6_walker list.

net->ipv6.fib6_walkers : 0xFFFFFFC013114DB0
next : 0xFFFFFF81E0899C88
prev : 0xFFFFFF81E0899C88

w->lh : 0xFFFFFF81E0899C88
next : 0xFFFFFF81E0899C88 // should be 0xFFFFFFC013114DB0
prev : 0xFFFFFF81E0899C88 // should be 0xFFFFFFC013114DB0

Looking over the seq_file operations for this, the only way I can see 
ipv6_route_seq_setup_walk() being called on a walker that has not been 
removed from the list with fib6_walker_unlink() is if 
ipv6_route_iter_active() returns false during ipv6_route_seq_stop(). As 
far as I can tell, this check is trying to assess if the walker has 
reached the end of the tree, and therefore no longer placed back on the 
fib6_walker list by ipv6_route_seq_next(), to avoid trying to double 
unlink the entry. This check seems to only be needed since 
fib6_unlink_walker() uses a plain list_del() call instead of 
list_del_init(), so simply checking list_empty(&w->lh) wouldn't 
correctly indicate if the walker had been unlinked previously.

At least from the semantics of the seq_file operations being used in 
seq_read(), it seems that there wouldn't be a reason to keep the walker 
around in the list after the completion of ipv6_route_seq_stop(), so I'm 
wondering if switching to this more direct check would be appropriate, 
as there seems to be some way that the current roundabout method is 
failing.

Call trace:
   __list_add_valid+0x74/0x90
   fib6_walker_link+0x78/0xb8
   ipv6_route_seq_start+0xec/0x138
   seq_read+0x18c/0x5b8
   proc_reg_read+0xa4/0x188
   __vfs_read+0x60/0x204
   vfs_read+0xa4/0x144

Thanks,
Sean

                 reply	other threads:[~2021-01-14 23:48 UTC|newest]

Thread overview: [no followups] expand[flat|nested]  mbox.gz  Atom feed

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=a0d35019167c32728aa810fee91909b5@codeaurora.org \
    --to=stranche@codeaurora.org \
    --cc=dsahern@gmail.com \
    --cc=netdev@vger.kernel.org \
    --cc=subashab@codeaurora.org \
    --cc=weiwan@google.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).