Date: Tue, 18 Jul 2017 14:43:37 -0300 From: Luiz Otavio O Souza <lists.br@gmail.com> To: Vincenzo Maffione <v.maffione@gmail.com>, Marius Strobl <marius@freebsd.org>, hps@freebsd.org, ae@freebsd.org Cc: FreeBSD Net <freebsd-net@freebsd.org>, "Eggert, Lars" <lars@netapp.com> Subject: Re: NULL pointer dereference bug triggered by netmap Message-ID: <CAB=2f8zaFmF_mW3wq_zgp9R47_X4Q%2B=1c53qStUFm=vM8rRwXA@mail.gmail.com> In-Reply-To: <CA%2B_eA9jC6WFDk-96XDzCEZqea2_E3N8eL9_fHvevtVLXU40dHg@mail.gmail.com> References: <CA%2B_eA9goJ6j=q6UO-%2BfOt-aHVgFmujQfH8WfYEHf9=PQobdwHg@mail.gmail.com> <20170705110512.GA28058@alchemy.franken.de> <CA%2B_eA9h138BhHaeyEojzM3UFc__GWxV72uVSHCWVWw-6CbnGsw@mail.gmail.com> <20170711200510.GB60651@alchemy.franken.de> <CA%2B_eA9jC6WFDk-96XDzCEZqea2_E3N8eL9_fHvevtVLXU40dHg@mail.gmail.com>
next in thread | previous in thread | raw e-mail | index | archive | help
[-- Attachment #1 --] On 12 July 2017 at 02:19, Vincenzo Maffione wrote: > Yes. > > Actually, we would also need one beteween the following two options: > 1) Implementing a dummy if_start() for if_loop.c > 2) Prevent netmap from using if_loop. Hi, Please, check the attached patches. Luiz > > 2017-07-11 22:05 GMT+02:00 Marius Strobl <marius@freebsd.org>: > >> On Thu, Jul 06, 2017 at 02:19:42PM -0700, Vincenzo Maffione wrote: >> > Sure, can anyone commit this? >> >> The addition of KASSERTs like the below one to if_handoff() and >> if_start()? Sure. >> >> Marius >> >> > >> > Il 5 lug 2017 4:05 AM, "Marius Strobl" <marius@freebsd.org> ha scritto: >> > >> > > On Mon, Jul 03, 2017 at 05:08:09PM +0200, Vincenzo Maffione wrote: >> > > > Details here: >> > > > >> > > > https://github.com/luigirizzo/netmap/issues/322 >> > > > >> > > > Is it acceptable to commit the proposed patch? >> > > >> > > As suggested by hselasky@, the outliner problem at hand is better >> solved >> > > by a dummy if_start method in order to not hurt the fast-path. Thus, if >> > > anything at all, a KASSERT(ifp->if_start != NULL, "no if_start method") >> > > should be added to if_handoff() and if_start(). [-- Attachment #2 --] Index: sys/net/if_loop.c =================================================================== --- sys/net/if_loop.c (revision 320674) +++ sys/net/if_loop.c (working copy) @@ -104,6 +104,17 @@ static struct if_clone *lo_cloner; static const char loname[] = "lo"; +/* if_loop do not support packets comming from if_transmit()/if_start(). */ +static int +lo_if_transmit(struct ifnet *ifp, struct mbuf *m) +{ + + KASSERT(m == NULL, ("%s: if_transmit() not supported.", __func__)); + m_freem(m); + + return (ENOBUFS); +} + static void lo_clone_destroy(struct ifnet *ifp) { @@ -137,6 +148,7 @@ IFCAP_HWCSUM | IFCAP_HWCSUM_IPV6; ifp->if_hwassist = LO_CSUM_FEATURES | LO_CSUM_FEATURES6; if_attach(ifp); + if_settransmitfn(ifp, lo_if_transmit); bpfattach(ifp, DLT_NULL, sizeof(u_int32_t)); if (V_loif == NULL) V_loif = ifp; [-- Attachment #3 --] Index: sys/dev/netmap/netmap_generic.c =================================================================== --- sys/dev/netmap/netmap_generic.c (revision 320674) +++ sys/dev/netmap/netmap_generic.c (working copy) @@ -75,6 +75,7 @@ #include <sys/socket.h> /* sockaddrs */ #include <sys/selinfo.h> #include <net/if.h> +#include <net/if_types.h> #include <net/if_var.h> #include <machine/bus.h> /* bus_dmamap_* in netmap_kern.h */ @@ -1198,6 +1199,13 @@ int retval; u_int num_tx_desc, num_rx_desc; +#ifdef __FreeBSD__ + if (ifp->if_type == IFT_LOOP) { + D("if_loop is not supported by %s", __func__); + return EINVAL; + } +#endif + num_tx_desc = num_rx_desc = netmap_generic_ringsize; /* starting point */ nm_os_generic_find_num_desc(ifp, &num_tx_desc, &num_rx_desc); /* ignore errors */
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?CAB=2f8zaFmF_mW3wq_zgp9R47_X4Q%2B=1c53qStUFm=vM8rRwXA>