Date: Thu, 19 May 2016 17:16:13 -0400 From: Steve Kiernan <stevek@juniper.net> To: Raviprakash Darbha <rdarbha@juniper.net> Cc: "freebsd-net@freebsd.org" <freebsd-net@freebsd.org>, "andre@freebsd.org" <andre@freebsd.org> Subject: Re: Double lock issue of unp_link_rwlock in usrreq.c observed Message-ID: <20160519171613.2da53612@stevek-ubuntu.jnpr.net> In-Reply-To: <948AD75B-BF6E-4672-8B50-9CF9E25667EA@juniper.net> References: <948AD75B-BF6E-4672-8B50-9CF9E25667EA@juniper.net>
next in thread | previous in thread | raw e-mail | index | archive | help
On Thu, 19 May 2016 17:06:34 -0400
Raviprakash Darbha <rdarbha@juniper.net> wrote:
> Hello Andre=20
>=20
> I encountered a double lock issue in unp_connectat function. After lookin=
g at the code , I think the unp_link_rwlock is being locked once unp_connec=
tat 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.
>=20
>=20
Just to clarify, this is uipc_connect() at this point...
> UNP_LINK_WLOCK(); <=E2=80=94=E2=80=94=E2=80=94=E2=80=94=E2=80=94=
=E2=80=94=E2=80=94=E2=80=94=E2=80=94=E2=80=94 1 st call=20
> =E2=80=A6..
> =E2=80=A6..
...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);=20
> so3 =3D sonewconn(so2, 0);
> // Expanding sonewconn=20
> {=20
> sonewconn=20
> {
> =E2=80=A6=E2=80=A6
> soalloc
> =E2=80=A6=E2=80=A6.
> pru_attach=20
> =E2=80=A6=E2=80=A6.
> if (!(head->so_options & SO_ACCEPTCONN) &&
> ((head->so_proto->pr_protocol !=3D IPPROTO_SCTP) ||
> (head->so_type !=3D SOCK_SEQPACKET))) {
> =E2=80=A6=E2=80=A6=E2=80=A6.
> sofree(so); /* NB: returns ACCEPT_UNLO=
CK'ed. */
>=20
> // Expanding sofree=20
>=20
> { =20
>=20
> =E2=80=A6=E2=80=A6.
>=20
> pru_detach
>=20
> // expanding pru_detach=20
>=20
> {
>=20
> // Recursive wlock acquiring.=20
>=20
> UNP_LINK_WLOCK() <=E2=80=94=E2=80=94=E2=
=80=94=E2=80=94=E2=80=94=E2=80=94=E2=80=94=E2=80=94=E2=80=94=E2=80=94 2nd =
Call=20
>=20
> Let me know your thoughts or if you need more information. Thanks ! =20
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.
--=20
Steve Kiernan
Principal Engineer, Core OS/Kernel Group
Juniper Networks, Inc.
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?20160519171613.2da53612>