netdev - Re: [RFC 3/3] mac80211: support bpf monitor filter

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, 12 Apr 2017 17:25:29 +0200
From:   Johannes Berg <johannes@...solutions.net>
To:     David Miller <davem@...emloft.net>
Cc:     linux-wireless@...r.kernel.org, netdev@...r.kernel.org
Subject: Re: [RFC 3/3] mac80211: support bpf monitor filter

On Wed, 2017-04-12 at 11:22 -0400, David Miller wrote:
> From: Johannes Berg <johannes@...solutions.net>
> Date: Wed, 12 Apr 2017 16:29:07 +0200
> 
> > On Wed, 2017-04-12 at 13:07 +0200, Johannes Berg wrote:
> >> 
> >>  struct ieee80211_if_mntr {
> >>      u32 flags;
> >> 
> > [...]
> > +     bool deliver;
> > 
> > That's ... broken for multi-queue RX. I haven't really found a good
> > other way to do it. The best way will likely be to copy the SKB the
> > first time it's needed, build the radiotap header, and then keep a
> > reference to it to be able to clone it later if it's needed again.
> 
> If you don't recurse into the receive path for different devices
> before you are done with this boolean, simply make a global per-cpu
> boolean and use that.

No, that won't work. We don't recurse, but this is a per-interface
bool, as we can have multiple monitor interfaces (possibly with
different filters).

The problem comes from the fact that I did

for_each_interface()
	iface.deliver = run_bpf_program();

if (nobody_wanted_it)
	return;

skb = build_monitor_skb()

for_each_interface()
	if (iface.monitor)
		deliver(skb);


What I should be doing is something like this:

for_each_interface() {
	if (run_bpf_program()) {
		if (!skb)
			skb = build_monitor_skb();
		deliver(skb);
	}
}

where deliver() does skb_clone() internally or so.

johannes