Subject: Re: [PATCH] rds: fix use-after-free read in rds_find_bound

From: Sowmini Varadhan <[email-masked]>
Date: Sat, 30 Dec 2017 17:32:38 -0500

On (12/30/17 13:37), [email-masked] wrote:
> Well thats what the report says o.w flag test wouldn't have
> been attempted.

the bug report says "use-after-free". 

It doesnt say that rds_rs_to_sk(rs) is null (if rds_rs_to_sk(rs) was null,
rs would also be null, please cscope struct rds_sock)

What the bug report says is 
" The buggy address belongs to the object at ffff8801c09a6080
  which belongs to the cache RDS of size 1472
 The buggy address is located 96 bytes inside of .."

96 is the offset of sk->sk_flags. so yes, there is a socket refcount
issue.

But the patch you sent (see next two lines) will not solve that.

> >>-	if (rs && !sock_flag(rds_rs_to_sk(rs), SOCK_DEAD))
> >>+	if (rs && rds_rs_to_sk(rs) && !sock_flag(rds_rs_to_sk(rs), SOCK_DEAD))

Sowmini>I think the real issue is refcount bug somewhere,

> Thats what I thought as well initially but since the reported case,
> the rs seems to be valid where as sk seems to be freed up as part of
> sock_release callback.

I dont understand the statement above- how can "rs be valid, and sk
be freed"?

rs_sk is embedded in the struct rds_sock, it is not a pointer. 

let's find and fix the refcount bug. See stack trace in commit comment.
The socket release is happening prematurely and existing WARN_ONs
are not catching it.

> >Was the syzbot test run with http://patchwork.ozlabs.org/patch/852492/
> >this sounds like that type of bug.

--Sowmini