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/f16f4a71154a
branches:  netbsd-8
changeset: 435111:f16f4a71154a
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 11632f72c273 -r f16f4a71154a 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