netdev - Re: [patch net] team: loadbalance: push lacpdus to exact delivery

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] [day] [month] [year] [list]

Date:   Fri, 26 Aug 2016 08:12:37 +0200
From:   Jiri Pirko <jiri@...nulli.us>
To:     Mahesh Bandewar
         (महेश बंडेवार) <maheshb@...gle.com>
Cc:     linux-netdev <netdev@...r.kernel.org>,
        David Miller <davem@...emloft.net>, idosch@...lanox.com,
        eladr@...lanox.com, yotamg@...lanox.com
Subject: Re: [patch net] team: loadbalance: push lacpdus to exact delivery

Thu, Aug 25, 2016 at 07:35:27PM CEST, maheshb@...gle.com wrote:
>On Thu, Aug 25, 2016 at 9:46 AM, Jiri Pirko <jiri@...nulli.us> wrote:
>> Thu, Aug 25, 2016 at 06:37:35PM CEST, maheshb@...gle.com wrote:
>>>On Thu, Aug 25, 2016 at 9:23 AM, Jiri Pirko <jiri@...nulli.us> wrote:
>>>> From: Jiri Pirko <jiri@...lanox.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 <idosch@...lanox.com>
>>>> Fixes: 8626c56c8279b ("bridge: fix potential use-after-free when hook returns QUEUE or STOLEN verdict")
>>>> Signed-off-by: Jiri Pirko <jiri@...lanox.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.

Let's be conservative in -net and possibly fix change this as a
follow-up in net-next

>
>>
>>>> +       }
>>>> +       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
>>>>