On 12/30/17 12:26 PM, Sowmini Varadhan wrote:
On (12/30/17 11:36), Santosh Shilimkar wrote:

socket buffer can get freed as part of sock_close
callback so before adding reference check underneath
socket validity.

I'm not sure I understand this fix-

struct rds_sock is:
   struct rds_sock {
         struct sock             rs_sk;
          :
   }

How can  rs be non-null but rds_rs_to_sk() is null? (Note that
rds_rs_to_sk just returns &rs->rs_sk) so the changed line is
identical to the original line.

Well thats what the report says o.w flag test wouldn't have
been attempted.

-       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))

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.

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

That fix scenario, the rs don't get inserted in hash table and
in this particular bug, the lookup was successful so am not sure
if these two bugs are related.

But since bound address fix was still not part of the build
reproduced use after free bug, $subject fix can wait for next
reproduction. Unfortunately as per the report, there is no
reproducer for it to test if other fix fixes this issue.

Regards,
Santosh

Reply via email to