Re: Crash due to destroying TCP request sockets using SOCK_DESTROY

quoted

Looks like for a TCP_NEW_SYN_RECV socket, sock_diag_destroy
essentially ends up doing:

                        struct request_sock *req = inet_reqsk(sk);

                        local_bh_disable();
                        
inet_csk_reqsk_queue_drop_and_put(req->rsk_listener,
                                                          req);
                        local_bh_enable();
...

        sock_gen_put(sk);

It looks like inet_csk_reqsk_queue_drop_and_put calls reqsk_put(req),
which frees the socket, and at that point sock_gen_put is a UAF. Do we
just need:

-                        
inet_csk_reqsk_queue_drop_and_put(req->rsk_listener,
-                                                           req);
+                        inet_csk_reqsk_queue_drop(req->rsk_listener, 
req);

since sock_gen_put will also end up calling reqsk_put() for a
TCP_SYN_RECV socket?

Alastair - you're able to reproduce this UAF using net_test on qemu,
right? If so, could you try that two-line patch above?

Hi Lorenzo

Your patch makes sense to me, please submit it formally with :

Fixes: d7226c7a4dd1 ("net: diag: Fix refcnt leak in error path
destroying socket")
Cc: David Ahern <redacted>

Thanks !

Thanks Lorenzo and Eric. I will try it out locally.

-- 
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
a Linux Foundation Collaborative Project