Source-Changes-HG archive
[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index][Old Index]
[src/trunk]: src/sys/net Fix not to use PPPOE_UNLOCK before acccess to pppoe_...
details: https://anonhg.NetBSD.org/src/rev/d11de93b3bee
branches: trunk
changeset: 323514:d11de93b3bee
user: yamaguchi <yamaguchi%NetBSD.org@localhost>
date: Mon Jun 18 09:49:05 2018 +0000
description:
Fix not to use PPPOE_UNLOCK before acccess to pppoe_softc
to avoid a race condition
According to the locking order of pppoe(4), the access to
pppoe_softc has to follow 5 steps as below.
1. aquire pppoe_softc_list_lock
2. aquire pppoe_softc lock
3. release pppoe_softc_list_lock
4. access to pppoe_softc
5. release pppoe_softc lock
However, pppoe_dispatch_disc_pkt() releases the lock of pppoe_softc
temporarily, and then re-aquires it before step 4 of the adove. So,
it is possible for other contexts to destroy a pppoe_softc in the
interim.
To fix this condition, avoid PPPOE_UNLOCK with the problem.
ok by knakahara@n.o
diffstat:
sys/net/if_pppoe.c | 118 ++++++++++++++++++++++++++++------------------------
1 files changed, 64 insertions(+), 54 deletions(-)
diffs (228 lines):
diff -r 61938b678016 -r d11de93b3bee sys/net/if_pppoe.c
--- a/sys/net/if_pppoe.c Mon Jun 18 09:12:17 2018 +0000
+++ b/sys/net/if_pppoe.c Mon Jun 18 09:49:05 2018 +0000
@@ -1,4 +1,4 @@
-/* $NetBSD: if_pppoe.c,v 1.138 2018/05/25 04:40:27 ozaki-r Exp $ */
+/* $NetBSD: if_pppoe.c,v 1.139 2018/06/18 09:49:05 yamaguchi Exp $ */
/*-
* Copyright (c) 2002, 2008 The NetBSD Foundation, Inc.
@@ -30,7 +30,7 @@
*/
#include <sys/cdefs.h>
-__KERNEL_RCSID(0, "$NetBSD: if_pppoe.c,v 1.138 2018/05/25 04:40:27 ozaki-r Exp $");
+__KERNEL_RCSID(0, "$NetBSD: if_pppoe.c,v 1.139 2018/06/18 09:49:05 yamaguchi Exp $");
#ifdef _KERNEL_OPT
#include "pppoe.h"
@@ -518,15 +518,15 @@
size_t ac_cookie_len;
uint8_t *relay_sid;
size_t relay_sid_len;
-#ifdef PPPOE_SERVER
uint8_t *hunique;
size_t hunique_len;
-#endif
struct pppoehdr *ph;
struct pppoetag *pt;
struct mbuf *n;
int noff, err, errortag;
struct ether_header *eh;
+ struct ifnet *rcvif;
+ struct psref psref;
/* as long as we don't know which instance */
strlcpy(devname, "pppoe", sizeof(devname));
@@ -545,10 +545,8 @@
ac_cookie_len = 0;
relay_sid = NULL;
relay_sid_len = 0;
-#ifdef PPPOE_SERVER
hunique = NULL;
hunique_len = 0;
-#endif
session = 0;
if (m->m_pkthdr.len - off <= PPPOE_HEADERLEN) {
printf("pppoe: packet too short: %d\n", m->m_pkthdr.len);
@@ -601,8 +599,7 @@
case PPPOE_TAG_SNAME:
break; /* ignored */
case PPPOE_TAG_ACNAME:
- error = NULL;
- if (sc != NULL && len > 0) {
+ if (len > 0) {
error = malloc(len + 1, M_TEMP, M_NOWAIT);
if (error == NULL)
break;
@@ -616,40 +613,24 @@
}
strlcpy(error, mtod(n, char*) + noff, len + 1);
- printf("%s: connected to %s\n", devname, error);
+ printf("pppoe: connected to %s\n", error);
free(error, M_TEMP);
}
break; /* ignored */
- case PPPOE_TAG_HUNIQUE: {
- struct ifnet *rcvif;
- struct psref psref;
+ case PPPOE_TAG_HUNIQUE:
+ if (hunique == NULL) {
+ n = m_pulldown(m, off + sizeof(*pt), len,
+ &noff);
+ if (!n) {
+ m = NULL;
+ err_msg = "TAG HUNIQUE ERROR";
+ break;
+ }
- if (sc != NULL)
- break;
- n = m_pulldown(m, off + sizeof(*pt), len, &noff);
- if (!n) {
- m = NULL;
- err_msg = "TAG HUNIQUE ERROR";
- break;
- }
-#ifdef PPPOE_SERVER
- hunique = mtod(n, uint8_t *) + noff;
- hunique_len = len;
-#endif
- rcvif = m_get_rcvif_psref(m, &psref);
- if (rcvif != NULL) {
- sc = pppoe_find_softc_by_hunique(
- mtod(n, char *) + noff, len, rcvif,
- RW_READER);
- }
- m_put_rcvif_psref(rcvif, &psref);
- if (sc != NULL) {
- strlcpy(devname, sc->sc_sppp.pp_if.if_xname,
- sizeof(devname));
- PPPOE_UNLOCK(sc);
+ hunique = mtod(n, uint8_t *) + noff;
+ hunique_len = len;
}
break;
- }
case PPPOE_TAG_ACCOOKIE:
if (ac_cookie == NULL) {
n = m_pulldown(m, off + sizeof(*pt), len,
@@ -768,9 +749,6 @@
#endif /* PPPOE_SERVER */
case PPPOE_CODE_PADR:
#ifdef PPPOE_SERVER
- {
- struct ifnet *rcvif;
- struct psref psref;
/*
* get sc from ac_cookie if IFF_PASSIVE
*/
@@ -820,12 +798,24 @@
sc->sc_sppp.pp_up(&sc->sc_sppp);
break;
- }
#else
/* ignore, we are no access concentrator */
goto done;
#endif /* PPPOE_SERVER */
case PPPOE_CODE_PADO:
+ rcvif = m_get_rcvif_psref(m, &psref);
+ if (__predict_false(rcvif == NULL))
+ goto done;
+
+ if (hunique != NULL) {
+ sc = pppoe_find_softc_by_hunique(hunique,
+ hunique_len,
+ rcvif,
+ RW_WRITER);
+ }
+
+ m_put_rcvif_psref(rcvif, &psref);
+
if (sc == NULL) {
/* be quiet if there is not a single pppoe instance */
if (!LIST_EMPTY(&pppoe_softc_list))
@@ -833,8 +823,6 @@
goto done;
}
- PPPOE_LOCK(sc, RW_WRITER);
-
if (sc->sc_state != PPPOE_STATE_PADI_SENT) {
printf("%s: received unexpected PADO\n",
sc->sc_sppp.pp_if.if_xname);
@@ -889,11 +877,22 @@
PPPOE_UNLOCK(sc);
break;
case PPPOE_CODE_PADS:
+ rcvif = m_get_rcvif_psref(m, &psref);
+ if (__predict_false(rcvif == NULL))
+ goto done;
+
+ if (hunique != NULL) {
+ sc = pppoe_find_softc_by_hunique(hunique,
+ hunique_len,
+ rcvif,
+ RW_WRITER);
+ }
+
+ m_put_rcvif_psref(rcvif, &psref);
+
if (sc == NULL)
goto done;
- PPPOE_LOCK(sc, RW_WRITER);
-
sc->sc_session = session;
callout_stop(&sc->sc_timeout);
if (sc->sc_sppp.pp_if.if_flags & IFF_DEBUG)
@@ -904,26 +903,37 @@
sc->sc_sppp.pp_up(&sc->sc_sppp); /* notify upper layers */
break;
- case PPPOE_CODE_PADT: {
- struct ifnet *rcvif;
- struct psref psref;
-
+ case PPPOE_CODE_PADT:
rcvif = m_get_rcvif_psref(m, &psref);
- if (__predict_true(rcvif != NULL)) {
- sc = pppoe_find_softc_by_session(session, rcvif,
- RW_WRITER);
- }
+ if (__predict_false(rcvif == NULL))
+ goto done;
+
+ sc = pppoe_find_softc_by_session(session, rcvif,
+ RW_WRITER);
+
m_put_rcvif_psref(rcvif, &psref);
+
if (sc == NULL)
goto done;
pppoe_clear_softc(sc, "received PADT");
PPPOE_UNLOCK(sc);
break;
- }
default:
+ rcvif = m_get_rcvif_psref(m, &psref);
+ if (__predict_false(rcvif == NULL))
+ goto done;
+
+ if (hunique != NULL) {
+ sc = pppoe_find_softc_by_hunique(hunique,
+ hunique_len,
+ rcvif,
+ RW_READER);
+ }
+
+ m_put_rcvif_psref(rcvif, &psref);
+
if (sc != NULL) {
- PPPOE_LOCK(sc, RW_READER);
strlcpy(devname, sc->sc_sppp.pp_if.if_xname,
sizeof(devname));
PPPOE_UNLOCK(sc);
Home |
Main Index |
Thread Index |
Old Index