Re: use-after-free in sock_wake_async

[Eric Dumazet <eric.dumazet@gmail.com>]

On Thu, 2015-11-26 at 14:32 +0100, Hannes Frederic Sowa wrote:
> Hannes Frederic Sowa <hannes@stressinduktion.org> writes:
> 
> 
> > I have seen filesystems already doing so in .destroy_inode, that's why I
> > am asking. The allocation happens the same way as we do with sock_alloc,
> > e.g. shmem. I actually thought that struct inode already provides an
> > rcu_head for exactly that reason.
> 
> E.g.:

> +static void sock_destroy_inode(struct inode *inode)
> +{
> +	call_rcu(&inode->i_rcu, sock_cache_free_rcu);
> +}

I guess you missed few years back why we had to implement
SLAB_DESTROY_BY_RCU for TCP sockets to not destroy performance.

By adding RCU grace period before reuse of this inode (about 640 bytes
today), you are asking the CPU to evict from its cache precious content,
and slow down some workloads, adding lot of ram pressure, as the cpu
allocating a TCP socket will have to populate its cache for a cold
inode.

The reason we put in a small object the RCU protected fields should be
pretty clear.

Do not copy code that people wrote in other layers without understanding
the performance implications.

Thanks.