Source-Changes-HG archive
[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index][Old Index]
[src/netbsd-8]: src/sys/net Pull up following revision(s) (requested by yamag...
details: https://anonhg.NetBSD.org/src/rev/a9b5c301170e
branches: netbsd-8
changeset: 320567:a9b5c301170e
user: martin <martin%NetBSD.org@localhost>
date: Thu Jul 12 15:11:56 2018 +0000
description:
Pull up following revision(s) (requested by yamaguchi in ticket #890):
sys/net/if_pppoe.c: revision 1.137
sys/net/if_pppoe.c: revision 1.139
sys/net/if_pppoe.c: revision 1.140
Drop early if there's no PPPoE interface. Otherwise it is easy for someone
to flood dmesg over the local subnet.
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
Fix to aquire pppoe_softc_list_lock before read and write the list
ok by knakahara@n.o
diffstat:
sys/net/if_pppoe.c | 181 +++++++++++++++++++++++++++++++++-------------------
1 files changed, 114 insertions(+), 67 deletions(-)
diffs (truncated from 352 to 300 lines):
diff -r 76897214dcf5 -r a9b5c301170e sys/net/if_pppoe.c
--- a/sys/net/if_pppoe.c Wed Jul 11 16:51:29 2018 +0000
+++ b/sys/net/if_pppoe.c Thu Jul 12 15:11:56 2018 +0000
@@ -1,4 +1,4 @@
-/* $NetBSD: if_pppoe.c,v 1.125.6.8 2018/06/07 17:42:25 martin Exp $ */
+/* $NetBSD: if_pppoe.c,v 1.125.6.9 2018/07/12 15:11:56 martin 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.125.6.8 2018/06/07 17:42:25 martin Exp $");
+__KERNEL_RCSID(0, "$NetBSD: if_pppoe.c,v 1.125.6.9 2018/07/12 15:11:56 martin Exp $");
#ifdef _KERNEL_OPT
#include "pppoe.h"
@@ -277,8 +277,11 @@
{
int error = 0;
- if (!LIST_EMPTY(&pppoe_softc_list))
+ rw_enter(&pppoe_softc_list_lock, RW_READER);
+ if (!LIST_EMPTY(&pppoe_softc_list)) {
+ rw_exit(&pppoe_softc_list_lock);
error = EBUSY;
+ }
if (error == 0) {
if_clone_detach(&pppoe_cloner);
@@ -338,9 +341,12 @@
sppp_attach(&sc->sc_sppp.pp_if);
bpf_attach(&sc->sc_sppp.pp_if, DLT_PPP_ETHER, 0);
+ rw_enter(&pppoe_softc_list_lock, RW_READER);
if (LIST_EMPTY(&pppoe_softc_list)) {
pfil_add_ihook(pppoe_ifattach_hook, NULL, PFIL_IFNET, if_pfil);
}
+ rw_exit(&pppoe_softc_list_lock);
+
if_register(&sc->sc_sppp.pp_if);
rw_init(&sc->sc_lock);
@@ -424,14 +430,18 @@
{
struct pppoe_softc *sc, *t;
- if (LIST_EMPTY(&pppoe_softc_list))
+ rw_enter(&pppoe_softc_list_lock, RW_READER);
+ if (LIST_EMPTY(&pppoe_softc_list)) {
+ rw_exit(&pppoe_softc_list_lock);
return NULL;
+ }
- if (len != sizeof sc)
+ if (len != sizeof sc) {
+ rw_exit(&pppoe_softc_list_lock);
return NULL;
+ }
memcpy(&t, token, len);
- rw_enter(&pppoe_softc_list_lock, RW_READER);
LIST_FOREACH(sc, &pppoe_softc_list, sc_list) {
if (sc == t) {
PPPOE_LOCK(sc, lock);
@@ -518,15 +528,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 +555,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 +609,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 +623,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,
@@ -722,9 +713,12 @@
* got service name, concentrator name, and/or host unique.
* ignore if we have no interfaces with IFF_PASSIVE|IFF_UP.
*/
- if (LIST_EMPTY(&pppoe_softc_list))
+ rw_enter(&pppoe_softc_list_lock, RW_READER);
+ if (LIST_EMPTY(&pppoe_softc_list)) {
+ rw_exit(&pppoe_softc_list_lock);
goto done;
- rw_enter(&pppoe_softc_list_lock, RW_READER);
+ }
+
LIST_FOREACH(sc, &pppoe_softc_list, sc_list) {
PPPOE_LOCK(sc, RW_WRITER);
if (!(sc->sc_sppp.pp_if.if_flags & IFF_UP)) {
@@ -768,9 +762,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
*/
@@ -790,8 +781,12 @@
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))
- printf("pppoe: received PADR but could not find request for it\n");
+ rw_enter(&pppoe_softc_list_lock, RW_READER);
+ if (!LIST_EMPTY(&pppoe_softc_list)) {
+ printf("pppoe: received PADR"
+ " but could not find request for it\n");
+ }
+ rw_exit(&pppoe_softc_list_lock);
goto done;
}
@@ -820,21 +815,35 @@
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))
- printf("pppoe: received PADO but could not find request for it\n");
+ rw_enter(&pppoe_softc_list_lock, RW_READER);
+ if (!LIST_EMPTY(&pppoe_softc_list)) {
+ printf("pppoe: received PADO"
+ " but could not find request for it\n");
+ }
+ rw_exit(&pppoe_softc_list_lock);
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 +898,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 +924,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,
Home |
Main Index |
Thread Index |
Old Index