List corruption from ipv6_route_seq_start
From: <hidden>
Date: 2021-01-14 23:48:44
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