Double lock issue of unp_link_rwlock in usrreq.c observed

[Steve Kiernan]

On Thu, 19 May 2016 17:06:34 -0400
Raviprakash Darbha <rdarbha at juniper.net> wrote:

> Hello Andre 
> 
> I encountered a double lock issue in unp_connectat function. After looking at the code , I think the unp_link_rwlock is being locked once unp_connectat and once again in unp_detach  (called from sofree ). Would like to get your opinion on the issue and the fix. Below is the exact call stack.
> 
> 

Just to clarify, this is uipc_connect() at this point...

> UNP_LINK_WLOCK();         <——————————  1 st call 
> …..
> …..

...and unp_connectat() at this point.

> if (so->so_proto->pr_flags & PR_CONNREQUIRED) {
>      if (so2->so_options & SO_ACCEPTCONN
>          CURVNET_SET(so2->so_vnet); 
>           so3 = sonewconn(so2, 0);
>           // Expanding sonewconn 
>           { 
>              sonewconn 
>               {
>                    ……
>                    soalloc
>                    …….
>                    pru_attach 
>                    …….
>                    if (!(head->so_options & SO_ACCEPTCONN) &&
>                    ((head->so_proto->pr_protocol != IPPROTO_SCTP) ||
>                     (head->so_type != SOCK_SEQPACKET))) {
>                        ……….
>                        sofree(so);             /* NB: returns ACCEPT_UNLOCK'ed. */
> 
>                        // Expanding sofree 
> 
>                       {   
> 
>                         …….
> 
>                         pru_detach
> 
>                         // expanding pru_detach 
> 
>                         {
> 
>                              // Recursive wlock acquiring. 
> 
>                              UNP_LINK_WLOCK()     <——————————  2nd Call 
> 
> Let me know your thoughts or if you need more information. Thanks !  

I suspect that this UNP_LINK_WLOCK() should become an assert.

However, I'm not clear on the implications of removing the UNP_LINK_WUNLOCK() farther down in pru_detach, then.
Considering there's potentially a taskqueue call and vn_rele, depending on what is set in the unpcb structure.

-- 
Steve Kiernan
Principal Engineer, Core OS/Kernel Group
Juniper Networks, Inc.