netdev - Re: [PATCH 1/2] net/mlx5: Avoid panic when setting vport mac, getting vport config

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]
Message-ID: <000ab7af-da71-7c93-13fb-2e82639a1295@mellanox.com>
Date:   Mon, 4 Mar 2019 07:06:47 +0000
From:   Roi Dayan <roid@...lanox.com>
To:     Tonghao Zhang <xiangxia.m.yue@...il.com>
CC:     Saeed Mahameed <saeedm@...lanox.com>,
        "gerlitz.or@...il.com" <gerlitz.or@...il.com>,
        "davem@...emloft.net" <davem@...emloft.net>,
        "netdev@...r.kernel.org" <netdev@...r.kernel.org>,
        Eli Cohen <eli@...lanox.com>
Subject: Re: [PATCH 1/2] net/mlx5: Avoid panic when setting vport mac, getting
 vport config



On 04/03/2019 03:04, Tonghao Zhang wrote:
> On Sun, Mar 3, 2019 at 8:42 PM Roi Dayan <roid@...lanox.com> wrote:
>>
>>
>>
>> On 03/03/2019 11:56, xiangxia.m.yue@...il.com wrote:
>>> From: Tonghao Zhang <xiangxia.m.yue@...il.com>
>>>
>>> If we try to set VFs mac address on a VF (not PF) net device,
>>> the kernel will be crash. The commands are show as below:
>>>
>>> $ echo 2 > /sys/class/net/$MLX_PF0/device/sriov_numvfs
>>> $ ip link set $MLX_VF0 vf 0 mac 00:11:22:33:44:00
>>>
>>> [exception RIP: mlx5_eswitch_set_vport_mac+41]
>>> [ffffb8b7079e3688] do_setlink at ffffffff8f67f85b
>>> [ffffb8b7079e37a8] __rtnl_newlink at ffffffff8f683778
>>> [ffffb8b7079e3b68] rtnl_newlink at ffffffff8f683a63
>>> [ffffb8b7079e3b90] rtnetlink_rcv_msg at ffffffff8f67d812
>>> [ffffb8b7079e3c10] netlink_rcv_skb at ffffffff8f6b88ab
>>> [ffffb8b7079e3c60] netlink_unicast at ffffffff8f6b808f
>>> [ffffb8b7079e3ca0] netlink_sendmsg at ffffffff8f6b8412
>>> [ffffb8b7079e3d18] sock_sendmsg at ffffffff8f6452f6
>>> [ffffb8b7079e3d30] ___sys_sendmsg at ffffffff8f645860
>>> [ffffb8b7079e3eb0] __sys_sendmsg at ffffffff8f647a38
>>> [ffffb8b7079e3f38] do_syscall_64 at ffffffff8f00401b
>>> [ffffb8b7079e3f50] entry_SYSCALL_64_after_hwframe at ffffffff8f80008c
>>>
>>> and
>>>
>>> [exception RIP: mlx5_eswitch_get_vport_config+12]
>>> [ffffa70607e57678] mlx5e_get_vf_config at ffffffffc03c7f8f [mlx5_core]
>>> [ffffa70607e57688] do_setlink at ffffffffbc67fa59
>>> [ffffa70607e577a8] __rtnl_newlink at ffffffffbc683778
>>> [ffffa70607e57b68] rtnl_newlink at ffffffffbc683a63
>>> [ffffa70607e57b90] rtnetlink_rcv_msg at ffffffffbc67d812
>>> [ffffa70607e57c10] netlink_rcv_skb at ffffffffbc6b88ab
>>> [ffffa70607e57c60] netlink_unicast at ffffffffbc6b808f
>>> [ffffa70607e57ca0] netlink_sendmsg at ffffffffbc6b8412
>>> [ffffa70607e57d18] sock_sendmsg at ffffffffbc6452f6
>>> [ffffa70607e57d30] ___sys_sendmsg at ffffffffbc645860
>>> [ffffa70607e57eb0] __sys_sendmsg at ffffffffbc647a38
>>> [ffffa70607e57f38] do_syscall_64 at ffffffffbc00401b
>>> [ffffa70607e57f50] entry_SYSCALL_64_after_hwframe at ffffffffbc80008c
>>>
>>> Fixes: a8d70a054a718 ("net/mlx5: E-Switch, Disallow vlan/spoofcheck setup if not being esw manager")
>>> Cc: Eli Cohen <eli@...lanox.com>
>>> Signed-off-by: Tonghao Zhang <xiangxia.m.yue@...il.com>
>>> ---
>>>  drivers/net/ethernet/mellanox/mlx5/core/eswitch.c | 6 ++++++
>>>  1 file changed, 6 insertions(+)
>>>
>>> diff --git a/drivers/net/ethernet/mellanox/mlx5/core/eswitch.c b/drivers/net/ethernet/mellanox/mlx5/core/eswitch.c
>>> index 6cb9710..774edc9 100644
>>> --- a/drivers/net/ethernet/mellanox/mlx5/core/eswitch.c
>>> +++ b/drivers/net/ethernet/mellanox/mlx5/core/eswitch.c
>>> @@ -1871,6 +1871,9 @@ int mlx5_eswitch_set_vport_mac(struct mlx5_eswitch *esw,
>>>       u64 node_guid;
>>>       int err = 0;
>>>
>>> +     if (!ESW_ALLOWED(esw))
>>> +             return -EPERM;
>>> +
>>
>> this will introduce a bug with smart nic.
>> from the commit in the fixes line, in smart nic the PF
>> is not an esw manager so it will block changing vf mac
>> with the pf. the fix should be checking if esw is null first.
> Thanks for your reply, I don't get the smart nic card and can't test
> it. So to fix this bug,
> we only check the esw is null right ?

correct. in smart nic we have PF and ECPF. PF is vport manager but not
esw manager and ECPF is the esw manager.
We set vf mac through the pf so the condition here should only be
vport group manager.


> 
> diff --git a/drivers/net/ethernet/mellanox/mlx5/core/eswitch.c
> b/drivers/net/ethernet/mellanox/mlx5/core/eswitch.c
> index 6cb9710..dc332ba 100644
> --- a/drivers/net/ethernet/mellanox/mlx5/core/eswitch.c
> +++ b/drivers/net/ethernet/mellanox/mlx5/core/eswitch.c
> @@ -1871,6 +1871,9 @@ int mlx5_eswitch_set_vport_mac(struct mlx5_eswitch *esw,
>         u64 node_guid;
>         int err = 0;
> 
> +       if (!esw)
> +               return -EPERM;
> +
>         if (!MLX5_CAP_GEN(esw->dev, vport_group_manager))
>                 return -EPERM;

maybe just add the condition to the same if.
         if (!esw || !MLX5_CAP_GEN(esw->dev, vport_group_manager))
                 return -EPERM;


>         if (!LEGAL_VPORT(esw, vport) || is_multicast_ether_addr(mac))
> @@ -1945,6 +1948,9 @@ int mlx5_eswitch_get_vport_config(struct
> mlx5_eswitch *esw,
>  {
>         struct mlx5_vport *evport;
> 
> +       if (!esw)
> +               return -EPERM;
> +
>         if (!MLX5_CAP_GEN(esw->dev, vport_group_manager))
>                 return -EPERM;
>         if (!LEGAL_VPORT(esw, vport))
> 
>>
>>>       if (!MLX5_CAP_GEN(esw->dev, vport_group_manager))
>>>               return -EPERM;
>>>       if (!LEGAL_VPORT(esw, vport) || is_multicast_ether_addr(mac))
>>> @@ -1945,6 +1948,9 @@ int mlx5_eswitch_get_vport_config(struct mlx5_eswitch *esw,
>>>  {
>>>       struct mlx5_vport *evport;
>>>
>>> +     if (!ESW_ALLOWED(esw))
>>> +             return -EPERM;
>>> +
>>>       if (!MLX5_CAP_GEN(esw->dev, vport_group_manager))
>>>               return -EPERM;
>>>       if (!LEGAL_VPORT(esw, vport))
>>>