netdev - Re: [net-next PATCH] qdisc: validate frames going through the direct

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:	Wed, 3 Sep 2014 16:26:28 +0200
From:	Jesper Dangaard Brouer <brouer@...hat.com>
To:	Jesper Dangaard Brouer <brouer@...hat.com>
Cc:	Eric Dumazet <eric.dumazet@...il.com>,
	"David S. Miller" <davem@...emloft.net>,
	Alexander Duyck <alexander.duyck@...il.com>,
	netdev@...r.kernel.org
Subject: Re: [net-next PATCH] qdisc: validate frames going through the
 direct_xmit path

On Wed, 3 Sep 2014 15:52:13 +0200
Jesper Dangaard Brouer <brouer@...hat.com> wrote:

> On Wed, 03 Sep 2014 06:43:25 -0700
> Eric Dumazet <eric.dumazet@...il.com> wrote:
> 
> > 
> > Jesper, you missed another spot, when there is no qdisc on the device.
> > 
> > __dev_queue_xmit() calls dev_hard_start_xmit() around line 2886
> > 
> > Could we try to not add a myriad of small patches ?
> > 
> > Some of us will need to backport all of them.
> 
> I'm in the same backport situation ;-)
> 
> I'll send a V2 of this patch, with missed spot...

(to avoid doing too many patches)

In the code below (from __dev_queue_xmit()), I've added validate_xmit_skb().
(dev_hard_start_xmit() can handle if it is a NULL ptr)

	HARD_TX_LOCK(dev, txq, cpu);
	if (!netif_xmit_stopped(txq)) {
		skb = validate_xmit_skb(skb, dev);
		__this_cpu_inc(xmit_recursion);
		skb = dev_hard_start_xmit(skb, dev, txq, &rc);
		__this_cpu_dec(xmit_recursion);
		if (dev_xmit_complete(rc)) {
			HARD_TX_UNLOCK(dev, txq);
			goto out;
		}
	}
	HARD_TX_UNLOCK(dev, txq);
	net_crit_ratelimited("Virtual device %s asks to queue packet!\n",
					     dev->name);
BUT shouldn't we also handle if dev_hard_start_xmit() returns an skb pointer,
which means that there were skb's left on the skb list.  Due to the rc
value "returned" in this case, we should see the net_crit_ratelimited()
msg, but we don't handle freeing these SKBs.

Any recommentations?

-- 
Best regards,
  Jesper Dangaard Brouer
  MSc.CS, Sr. Network Kernel Developer at Red Hat
  Author of http://www.iptv-analyzer.org
  LinkedIn: http://www.linkedin.com/in/brouer
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html