From owner-svn-src-all@freebsd.org Thu Jan 25 00:20:42 2018 Return-Path: Delivered-To: svn-src-all@mailman.ysv.freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2610:1c1:1:606c::19:1]) by mailman.ysv.freebsd.org (Postfix) with ESMTP id 72CE4ECEF33; Thu, 25 Jan 2018 00:20:42 +0000 (UTC) (envelope-from glebius@FreeBSD.org) Received: from cell.glebi.us (glebi.us [96.95.210.25]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (Client CN "cell.glebi.us", Issuer "cell.glebi.us" (not verified)) by mx1.freebsd.org (Postfix) with ESMTPS id 080F37F48C; Thu, 25 Jan 2018 00:20:41 +0000 (UTC) (envelope-from glebius@FreeBSD.org) Received: from cell.glebi.us (localhost [127.0.0.1]) by cell.glebi.us (8.15.2/8.15.2) with ESMTPS id w0P0Kebs076091 (version=TLSv1.2 cipher=DHE-RSA-AES256-GCM-SHA384 bits=256 verify=NO); Wed, 24 Jan 2018 16:20:40 -0800 (PST) (envelope-from glebius@FreeBSD.org) Received: (from glebius@localhost) by cell.glebi.us (8.15.2/8.15.2/Submit) id w0P0Kew2076090; Wed, 24 Jan 2018 16:20:40 -0800 (PST) (envelope-from glebius@FreeBSD.org) X-Authentication-Warning: cell.glebi.us: glebius set sender to glebius@FreeBSD.org using -f Date: Wed, 24 Jan 2018 16:20:40 -0800 From: Gleb Smirnoff To: Michael Zhilin Cc: src-committers@freebsd.org, svn-src-all@freebsd.org, svn-src-head@freebsd.org Subject: Re: svn commit: r328377 - in head/sys/dev/etherswitch: arswitch e6000sw infineon ip17x micrel mtkswitch rtl8366 ukswitch Message-ID: <20180125002040.GK8113@FreeBSD.org> References: <201801242133.w0OLXIOe078281@repo.freebsd.org> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <201801242133.w0OLXIOe078281@repo.freebsd.org> User-Agent: Mutt/1.9.1 (2017-09-22) X-BeenThere: svn-src-all@freebsd.org X-Mailman-Version: 2.1.25 Precedence: list List-Id: "SVN commit messages for the entire src tree \(except for " user" and " projects" \)" List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Thu, 25 Jan 2018 00:20:42 -0000 Hi Michael, I already replied to Dmitry that this patch isn't correct. The if_alloc() shall never fail. All checks like that needs to be removed. Inside the if_alloc() the check for L2COM allocation should also be removed. Everything happens with M_WAITOK in this path. On Wed, Jan 24, 2018 at 09:33:18PM +0000, Michael Zhilin wrote: M> Author: mizhka M> Date: Wed Jan 24 21:33:18 2018 M> New Revision: 328377 M> URL: https://svnweb.freebsd.org/changeset/base/328377 M> M> Log: M> [etherswitch] check if_alloc returns NULL M> M> This patch is cosmetic. It checks if allocation of ifnet structure failed. M> It's better to have this check rather than assume positive scenario. M> M> Submitted by: Dmitry Luhtionov M> Reported by: Dmitry Luhtionov M> M> Modified: M> head/sys/dev/etherswitch/arswitch/arswitch.c M> head/sys/dev/etherswitch/e6000sw/e6060sw.c M> head/sys/dev/etherswitch/infineon/adm6996fc.c M> head/sys/dev/etherswitch/ip17x/ip17x.c M> head/sys/dev/etherswitch/micrel/ksz8995ma.c M> head/sys/dev/etherswitch/mtkswitch/mtkswitch.c M> head/sys/dev/etherswitch/rtl8366/rtl8366rb.c M> head/sys/dev/etherswitch/ukswitch/ukswitch.c M> M> Modified: head/sys/dev/etherswitch/arswitch/arswitch.c M> ============================================================================== M> --- head/sys/dev/etherswitch/arswitch/arswitch.c Wed Jan 24 21:26:01 2018 (r328376) M> +++ head/sys/dev/etherswitch/arswitch/arswitch.c Wed Jan 24 21:33:18 2018 (r328377) M> @@ -177,6 +177,12 @@ arswitch_attach_phys(struct arswitch_softc *sc) M> snprintf(name, IFNAMSIZ, "%sport", device_get_nameunit(sc->sc_dev)); M> for (phy = 0; phy < sc->numphys; phy++) { M> sc->ifp[phy] = if_alloc(IFT_ETHER); M> + if (sc->ifp[phy] == NULL) { M> + device_printf(sc->sc_dev, "couldn't allocate ifnet structure\n"); M> + err = ENOMEM; M> + break; M> + } M> + M> sc->ifp[phy]->if_softc = sc; M> sc->ifp[phy]->if_flags |= IFF_UP | IFF_BROADCAST | M> IFF_DRV_RUNNING | IFF_SIMPLEX; M> M> Modified: head/sys/dev/etherswitch/e6000sw/e6060sw.c M> ============================================================================== M> --- head/sys/dev/etherswitch/e6000sw/e6060sw.c Wed Jan 24 21:26:01 2018 (r328376) M> +++ head/sys/dev/etherswitch/e6000sw/e6060sw.c Wed Jan 24 21:33:18 2018 (r328377) M> @@ -218,6 +218,12 @@ e6060sw_attach_phys(struct e6060sw_softc *sc) M> sc->ifpport[phy] = port; M> sc->portphy[port] = phy; M> sc->ifp[port] = if_alloc(IFT_ETHER); M> + if (sc->ifp[port] == NULL) { M> + device_printf(sc->sc_dev, "couldn't allocate ifnet structure\n"); M> + err = ENOMEM; M> + break; M> + } M> + M> sc->ifp[port]->if_softc = sc; M> sc->ifp[port]->if_flags |= IFF_UP | IFF_BROADCAST | M> IFF_DRV_RUNNING | IFF_SIMPLEX; M> M> Modified: head/sys/dev/etherswitch/infineon/adm6996fc.c M> ============================================================================== M> --- head/sys/dev/etherswitch/infineon/adm6996fc.c Wed Jan 24 21:26:01 2018 (r328376) M> +++ head/sys/dev/etherswitch/infineon/adm6996fc.c Wed Jan 24 21:33:18 2018 (r328377) M> @@ -175,6 +175,12 @@ adm6996fc_attach_phys(struct adm6996fc_softc *sc) M> sc->ifpport[phy] = port; M> sc->portphy[port] = phy; M> sc->ifp[port] = if_alloc(IFT_ETHER); M> + if (sc->ifp[port] == NULL) { M> + device_printf(sc->sc_dev, "couldn't allocate ifnet structure\n"); M> + err = ENOMEM; M> + break; M> + } M> + M> sc->ifp[port]->if_softc = sc; M> sc->ifp[port]->if_flags |= IFF_UP | IFF_BROADCAST | M> IFF_DRV_RUNNING | IFF_SIMPLEX; M> M> Modified: head/sys/dev/etherswitch/ip17x/ip17x.c M> ============================================================================== M> --- head/sys/dev/etherswitch/ip17x/ip17x.c Wed Jan 24 21:26:01 2018 (r328376) M> +++ head/sys/dev/etherswitch/ip17x/ip17x.c Wed Jan 24 21:33:18 2018 (r328377) M> @@ -174,6 +174,12 @@ ip17x_attach_phys(struct ip17x_softc *sc) M> sc->phyport[phy] = port; M> sc->portphy[port] = phy; M> sc->ifp[port] = if_alloc(IFT_ETHER); M> + if (sc->ifp[port] == NULL) { M> + device_printf(sc->sc_dev, "couldn't allocate ifnet structure\n"); M> + err = ENOMEM; M> + break; M> + } M> + M> sc->ifp[port]->if_softc = sc; M> sc->ifp[port]->if_flags |= IFF_UP | IFF_BROADCAST | M> IFF_DRV_RUNNING | IFF_SIMPLEX; M> M> Modified: head/sys/dev/etherswitch/micrel/ksz8995ma.c M> ============================================================================== M> --- head/sys/dev/etherswitch/micrel/ksz8995ma.c Wed Jan 24 21:26:01 2018 (r328376) M> +++ head/sys/dev/etherswitch/micrel/ksz8995ma.c Wed Jan 24 21:33:18 2018 (r328377) M> @@ -221,6 +221,12 @@ ksz8995ma_attach_phys(struct ksz8995ma_softc *sc) M> sc->ifpport[phy] = port; M> sc->portphy[port] = phy; M> sc->ifp[port] = if_alloc(IFT_ETHER); M> + if (sc->ifp[port] == NULL) { M> + device_printf(sc->sc_dev, "couldn't allocate ifnet structure\n"); M> + err = ENOMEM; M> + break; M> + } M> + M> sc->ifp[port]->if_softc = sc; M> sc->ifp[port]->if_flags |= IFF_UP | IFF_BROADCAST | M> IFF_DRV_RUNNING | IFF_SIMPLEX; M> M> Modified: head/sys/dev/etherswitch/mtkswitch/mtkswitch.c M> ============================================================================== M> --- head/sys/dev/etherswitch/mtkswitch/mtkswitch.c Wed Jan 24 21:26:01 2018 (r328376) M> +++ head/sys/dev/etherswitch/mtkswitch/mtkswitch.c Wed Jan 24 21:33:18 2018 (r328377) M> @@ -122,6 +122,12 @@ mtkswitch_attach_phys(struct mtkswitch_softc *sc) M> continue; M> } M> sc->ifp[phy] = if_alloc(IFT_ETHER); M> + if (sc->ifp[phy] == NULL) { M> + device_printf(sc->sc_dev, "couldn't allocate ifnet structure\n"); M> + err = ENOMEM; M> + break; M> + } M> + M> sc->ifp[phy]->if_softc = sc; M> sc->ifp[phy]->if_flags |= IFF_UP | IFF_BROADCAST | M> IFF_DRV_RUNNING | IFF_SIMPLEX; M> M> Modified: head/sys/dev/etherswitch/rtl8366/rtl8366rb.c M> ============================================================================== M> --- head/sys/dev/etherswitch/rtl8366/rtl8366rb.c Wed Jan 24 21:26:01 2018 (r328376) M> +++ head/sys/dev/etherswitch/rtl8366/rtl8366rb.c Wed Jan 24 21:33:18 2018 (r328377) M> @@ -239,6 +239,12 @@ rtl8366rb_attach(device_t dev) M> /* PHYs need an interface, so we generate a dummy one */ M> for (i = 0; i < sc->numphys; i++) { M> sc->ifp[i] = if_alloc(IFT_ETHER); M> + if (sc->ifp[i] == NULL) { M> + device_printf(sc->sc_dev, "couldn't allocate ifnet structure\n"); M> + err = ENOMEM; M> + break; M> + } M> + M> sc->ifp[i]->if_softc = sc; M> sc->ifp[i]->if_flags |= IFF_UP | IFF_BROADCAST | IFF_DRV_RUNNING M> | IFF_SIMPLEX; M> M> Modified: head/sys/dev/etherswitch/ukswitch/ukswitch.c M> ============================================================================== M> --- head/sys/dev/etherswitch/ukswitch/ukswitch.c Wed Jan 24 21:26:01 2018 (r328376) M> +++ head/sys/dev/etherswitch/ukswitch/ukswitch.c Wed Jan 24 21:33:18 2018 (r328377) M> @@ -126,6 +126,12 @@ ukswitch_attach_phys(struct ukswitch_softc *sc) M> sc->ifpport[phy] = port; M> sc->portphy[port] = phy; M> sc->ifp[port] = if_alloc(IFT_ETHER); M> + if (sc->ifp[port] == NULL) { M> + device_printf(sc->sc_dev, "couldn't allocate ifnet structure\n"); M> + err = ENOMEM; M> + break; M> + } M> + M> sc->ifp[port]->if_softc = sc; M> sc->ifp[port]->if_flags |= IFF_UP | IFF_BROADCAST | M> IFF_DRV_RUNNING | IFF_SIMPLEX; M> -- Gleb Smirnoff