net: bridge: fix per-port af_packet sockets

When the commit below was introduced it changed two visible things:
 - the skb was no longer passed through the protocol handlers with the
   original device
 - the skb was passed up the stack with skb->dev = bridge

The first change broke af_packet sockets on bridge ports. For example we
use them for hostapd which listens for ETH_P_PAE packets on the ports.
We discussed two possible fixes:
 - create a clone and pass it through NF_HOOK(), act on the original skb
   based on the result
 - somehow signal to the caller from the okfn() that it was called,
   meaning the skb is ok to be passed, which this patch is trying to
   implement via returning 1 from the bridge link-local okfn()

Note that we rely on the fact that NF_QUEUE/STOLEN would return 0 and
drop/error would return < 0 thus the okfn() is called only when the
return was 1, so we signal to the caller that it was called by preserving
the return value from nf_hook().

Fixes: 8626c56c82 ("bridge: fix potential use-after-free when hook returns QUEUE or STOLEN verdict")
Signed-off-by: Nikolay Aleksandrov <nikolay@cumulusnetworks.com>
Signed-off-by: David S. Miller <davem@davemloft.net>
This commit is contained in:
Nikolay Aleksandrov 2019-04-11 13:56:39 +03:00 committed by David S. Miller
parent 614c70f35c
commit 3b2e2904de

View File

@ -197,13 +197,10 @@ static void __br_handle_local_finish(struct sk_buff *skb)
/* note: already called with rcu_read_lock */ /* note: already called with rcu_read_lock */
static int br_handle_local_finish(struct net *net, struct sock *sk, struct sk_buff *skb) static int br_handle_local_finish(struct net *net, struct sock *sk, struct sk_buff *skb)
{ {
struct net_bridge_port *p = br_port_get_rcu(skb->dev);
__br_handle_local_finish(skb); __br_handle_local_finish(skb);
BR_INPUT_SKB_CB(skb)->brdev = p->br->dev; /* return 1 to signal the okfn() was called so it's ok to use the skb */
br_pass_frame_up(skb); return 1;
return 0;
} }
/* /*
@ -280,10 +277,18 @@ rx_handler_result_t br_handle_frame(struct sk_buff **pskb)
goto forward; goto forward;
} }
/* Deliver packet to local host only */ /* The else clause should be hit when nf_hook():
NF_HOOK(NFPROTO_BRIDGE, NF_BR_LOCAL_IN, dev_net(skb->dev), * - returns < 0 (drop/error)
NULL, skb, skb->dev, NULL, br_handle_local_finish); * - returns = 0 (stolen/nf_queue)
return RX_HANDLER_CONSUMED; * Thus return 1 from the okfn() to signal the skb is ok to pass
*/
if (NF_HOOK(NFPROTO_BRIDGE, NF_BR_LOCAL_IN,
dev_net(skb->dev), NULL, skb, skb->dev, NULL,
br_handle_local_finish) == 1) {
return RX_HANDLER_PASS;
} else {
return RX_HANDLER_CONSUMED;
}
} }
forward: forward: