On Thu, Aug 25, 2016 at 9:46 AM, Jiri Pirko <j...@resnulli.us> wrote:
> Thu, Aug 25, 2016 at 06:37:35PM CEST, mahe...@google.com wrote:
>>On Thu, Aug 25, 2016 at 9:23 AM, Jiri Pirko <j...@resnulli.us> wrote:
>>> From: Jiri Pirko <j...@mellanox.com>
>>>
>>> When team is in bridge and LACP is utilized, LACPDU packets are pushed
>>> to userspace using raw socket and there they are processed. However,
>>> since 8626c56c8279b, LACPDU skbs are dropped by bridge rx_handler so
>>> they never reach packet handlers in rx path. Fix this by explicity treat
>>> LACPDUs to be pushed to exact delivery in team rx_handler.
>>>
>>> Reported-by: Ido Schimmel <ido...@mellanox.com>
>>> Fixes: 8626c56c8279b ("bridge: fix potential use-after-free when hook
>>> returns QUEUE or STOLEN verdict")
>>> Signed-off-by: Jiri Pirko <j...@mellanox.com>
>>> ---
>>> drivers/net/team/team_mode_loadbalance.c | 14 ++++++++++++++
>>> 1 file changed, 14 insertions(+)
>>>
>>> diff --git a/drivers/net/team/team_mode_loadbalance.c
>>> b/drivers/net/team/team_mode_loadbalance.c
>>> index cdb19b3..e7c8210 100644
>>> --- a/drivers/net/team/team_mode_loadbalance.c
>>> +++ b/drivers/net/team/team_mode_loadbalance.c
>>> @@ -17,6 +17,19 @@
>>> #include <linux/filter.h>
>>> #include <linux/if_team.h>
>>>
>>> +static rx_handler_result_t lb_receive(struct team *team, struct team_port
>>> *port,
>>> + struct sk_buff *skb)
>>> +{
>>> + if (unlikely(skb->protocol == htons(ETH_P_SLOW))) {
>>> + /* LACPDU packets should go to exact delivery */
>>> + const unsigned char *dest = eth_hdr(skb)->h_dest;
>>> +
>>> + if (is_link_local_ether_addr(dest) && dest[5] == 0x02)
>>> + return RX_HANDLER_EXACT;
>>I believe every link-local-frame should get this treatment whether
>>it's 802.3ad or otherwise (e.g. LLDP etc.), no?
>>BTW same should be true for bonding too (of course except LACP since
>>it handles/consumes it!)
>
> I think so as well, but stayed more conservative for this fix. LLDP case
> is handled in bridge rx_handler though...
>
bridge rx_handler when team is in bridge, otherwise these link-local
frames will be appear on master. So I guess it's a choice between
being conservative (and fix only LACP case as this fix does) or fix a
generic case.
>
>>> + }
>>> + return RX_HANDLER_ANOTHER;
>>> +}
>>> +
>>> struct lb_priv;
>>>
>>> typedef struct team_port *lb_select_tx_port_func_t(struct team *,
>>> @@ -652,6 +665,7 @@ static const struct team_mode_ops lb_mode_ops = {
>>> .port_enter = lb_port_enter,
>>> .port_leave = lb_port_leave,
>>> .port_disabled = lb_port_disabled,
>>> + .receive = lb_receive,
>>> .transmit = lb_transmit,
>>> };
>>>
>>> --
>>> 2.5.5
>>>
