NULL pointer dereference bug triggered by netmap

[Luiz Otavio O Souza]

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 at 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 at 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().
-------------- next part --------------
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;
-------------- next part --------------
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 */