| From foo@baz Tue 23 Apr 2019 05:35:42 PM CEST |
| From: Nikolay Aleksandrov <nikolay@cumulusnetworks.com> |
| Date: Thu, 11 Apr 2019 13:56:39 +0300 |
| Subject: net: bridge: fix per-port af_packet sockets |
| |
| From: Nikolay Aleksandrov <nikolay@cumulusnetworks.com> |
| |
| [ Upstream commit 3b2e2904deb314cc77a2192f506f2fd44e3d10d0 ] |
| |
| 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: 8626c56c8279 ("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> |
| Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org> |
| --- |
| net/bridge/br_input.c | 23 ++++++++++++++--------- |
| 1 file changed, 14 insertions(+), 9 deletions(-) |
| |
| --- a/net/bridge/br_input.c |
| +++ b/net/bridge/br_input.c |
| @@ -231,13 +231,10 @@ static void __br_handle_local_finish(str |
| /* note: already called with rcu_read_lock */ |
| 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_INPUT_SKB_CB(skb)->brdev = p->br->dev; |
| - br_pass_frame_up(skb); |
| - return 0; |
| + /* return 1 to signal the okfn() was called so it's ok to use the skb */ |
| + return 1; |
| } |
| |
| /* |
| @@ -308,10 +305,18 @@ rx_handler_result_t br_handle_frame(stru |
| goto forward; |
| } |
| |
| - /* Deliver packet to local host only */ |
| - NF_HOOK(NFPROTO_BRIDGE, NF_BR_LOCAL_IN, dev_net(skb->dev), |
| - NULL, skb, skb->dev, NULL, br_handle_local_finish); |
| - return RX_HANDLER_CONSUMED; |
| + /* The else clause should be hit when nf_hook(): |
| + * - returns < 0 (drop/error) |
| + * - returns = 0 (stolen/nf_queue) |
| + * 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: |