Re: KASAN: use-after-free Read in sock_release

[Al Viro]

On Wed, Nov 29, 2017 at 11:37:04AM -0800, Cong Wang wrote:

> > Allocated by task 31066:
> >  save_stack+0x43/0xd0 mm/kasan/kasan.c:447
> >  set_track mm/kasan/kasan.c:459 [inline]
> >  kasan_kmalloc+0xad/0xe0 mm/kasan/kasan.c:551
> >  kmem_cache_alloc_trace+0x136/0x750 mm/slab.c:3613
> >  kmalloc include/linux/slab.h:499 [inline]
> >  sock_alloc_inode+0xb4/0x300 net/socket.c:253
> >  alloc_inode+0x65/0x180 fs/inode.c:208
> >  new_inode_pseudo+0x69/0x190 fs/inode.c:890
> >  sock_alloc+0x41/0x270 net/socket.c:565
> >  __sock_create+0x148/0x850 net/socket.c:1225
> >  sock_create net/socket.c:1301 [inline]
> >  SYSC_socket net/socket.c:1331 [inline]
> >  SyS_socket+0xeb/0x200 net/socket.c:1311
> >  entry_SYSCALL_64_fastpath+0x1f/0x96
> >
> > Freed by task 3039:
> >  save_stack+0x43/0xd0 mm/kasan/kasan.c:447
> >  set_track mm/kasan/kasan.c:459 [inline]
> >  kasan_slab_free+0x71/0xc0 mm/kasan/kasan.c:524
> >  __cache_free mm/slab.c:3491 [inline]
> >  kfree+0xca/0x250 mm/slab.c:3806
> >  __rcu_reclaim kernel/rcu/rcu.h:190 [inline]
> >  rcu_do_batch kernel/rcu/tree.c:2758 [inline]
> >  invoke_rcu_callbacks kernel/rcu/tree.c:3012 [inline]
> >  __rcu_process_callbacks kernel/rcu/tree.c:2979 [inline]
> >  rcu_process_callbacks+0xe79/0x17d0 kernel/rcu/tree.c:2996
> >  __do_softirq+0x29d/0xbb2 kernel/softirq.c:285

IDGI.  We are running into the object pointed to by sock->wq
already freed, right?  So how the hell had we managed to _fetch_
the pointer in the first place?  Freeing had been scheduled
by
        wq = rcu_dereference_protected(ei->socket.wq, 1);  
        kfree_rcu(wq, rcu);
        kmem_cache_free(sock_inode_cachep, ei);
so we should have
	* sock_destroy_inode() run on another CPU while we are
in the middle of sock_release(), sock->wq fetched by sock_release(),
sock->wq fed to kfree_rcu() by sock_destroy_inode() *AND* freed
before sock_release() got around to dereferencing it.

	Not impossible to hit, but... why hadn't we run into
much wider window?  If that sock_destroy_inode() on another
CPU had gotten to the call right after that kfree_rcu(), we
would've seen use-after-free on attempt to fetch ->wq...

	And it goes without saying that sock_destroy_inode() should
not have happened in parallel to sock_release(), or, for that matter,
to anything else done to struct socket instance...