Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::3:1 as permitted sender) client-ip=2620:137:e000::3:1;
Message-ID: <5a89b15e-e4ca-4e06-9069-6f005c6884f2@intel.com>
Date:   Wed, 25 Oct 2023 11:06:25 +0200
User-Agent: Mozilla Thunderbird
Subject: Re: [PATCH net] net/mlx5e: fix double free of encap_header
Content-Language: en-US
To:     Dust Li <dust.li@linux.alibaba.com>,
        Saeed Mahameed <saeedm@nvidia.com>,
        Leon Romanovsky <leon@kernel.org>,
        "David S. Miller" <davem@davemloft.net>,
        Eric Dumazet <edumazet@google.com>,
        Jakub Kicinski <kuba@kernel.org>,
        "Paolo Abeni" <pabeni@redhat.com>
CC:     <netdev@vger.kernel.org>, <linux-rdma@vger.kernel.org>,
        <linux-kernel@vger.kernel.org>,
        Cruz Zhao <cruzzhao@linux.alibaba.com>,
        Tianchen Ding <dtcccc@linux.alibaba.com>
References: <20231025032712.79026-1-dust.li@linux.alibaba.com>
From:   Wojciech Drewek <wojciech.drewek@intel.com>
In-Reply-To: <20231025032712.79026-1-dust.li@linux.alibaba.com>
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: 7bit
MIME-Version: 1.0
X-MS-Exchange-AntiSpam-MessageData-ChunkCount: 1
X-MS-Exchange-AntiSpam-MessageData-0: =?utf-8?B?L0lwSGRIbDdwVVQvSlZDbC9QbUZwZmN4bExYUzhHOFA3eWI5UGpVTjhMZ05O?=
 =?utf-8?B?Qk53cjBGYUxZYVFtTlUyNTRaQmRKOTdoVmhBQm4xTE5RM3NMSTlTa2I0SGVs?=
 =?utf-8?B?RWFxeEVaUWdSb3IzZFUrelpKdGJnMENGL241OWRjVzhrTVdrTlB1N2lkT3RS?=
 =?utf-8?B?U1pjMjFRc3FmbWpreklFcXVGb2NpUHpaSGlOWmdCMXVlSnVic0J3U3dEb3RT?=
 =?utf-8?B?S1hqcVUwOG14Q05rOXRMZm9zZm4vMG8raHh4OE1zcWVqWGlOVmh0TmxWN0Va?=
 =?utf-8?B?M1Z4SEx5MnZ6VmttOW53aGpxL0dPMzhVd0ZQY2pwS3hlMHNZdXU1dU5zN0V1?=
 =?utf-8?B?UjhvQzN1akU4bGJWQ05FT0tWaWNKTW1Gc0hrdEpCaEZNeFpHb1hWUVo2bW9h?=
 =?utf-8?B?cTVrdDMvU0FaYk1EZlFkOVpKMnE4UEdIMEVBeGllWkVwRExqd280WC92L28y?=
 =?utf-8?B?NVFLY1REcXNoUjJMVGVMdG0xWjFLeW9vWGNOUzZVMU9halRKUHR1cmRPeG1k?=
 =?utf-8?B?SlFNazh6ZThDWnJaZG1Pb0tjMmZFcXlIT1U0QmNydUVMWWNLbFVDMHFBZnBi?=
 =?utf-8?B?U1BoeU9nUGVOY3pvQUxUYTdOM041L3ovbHc1b3BTa3ZrRGI0WUFpenNKZXBk?=
 =?utf-8?B?Kzg3NmNhYlhjN3dBeGdPTmJmclNSa0dodUpRR0VucTJMdjRKSmd0WUxXVmd1?=
 =?utf-8?B?Ylo4aVRLd2ZmdTd1WmR1clZTVFBIUlluZFZ0U2tmdDlQSHpVb3dDdXhDNkpm?=
 =?utf-8?B?b2NkRExjR0NNdmJxSEJQWVVhVm0zRUllR21RSDlLTlRHWklUZWNUVWN3N2tN?=
 =?utf-8?B?RTlsNkJCd1RSQWhmcmRCV2dnc2dJMko0clhhcW92OFlWT3crRlVsWmNSdXVl?=
 =?utf-8?B?MGZsUkVvV0c0V3ZLY2hnR1NJbVRVdytjbnhSUVdvZm9rWGIrdnRyam5hZEg5?=
 =?utf-8?B?dW1zRnc2WFNsWGdNb2I5dXVyeUhLQ0wxQmN3V3lBWUNZeVZmQzJSeEJXN1dS?=
 =?utf-8?B?Ni9mRyt1dlRCdTQ1bkFqbS8rSTBEcGY4M2t1V3BQYWFQYjhheVc5WmRSRStD?=
 =?utf-8?B?UFgvMXQwdWhaSmd4Y1VhZmhYY3hEWUZjM3paWHNJUDAxSmE5bWpuWkovMEti?=
 =?utf-8?B?UDFBNGVCSjVPbVB5c0JOU0pNcUkxaUxqQUpFbjZ1UERLNVZZSVhSSWVoNEJt?=
 =?utf-8?B?QWM0WFJENGtxbXV6anp1aFFKNWJFRkI4azFCZTR4RXVrSDN1M3NtZUhuNGo1?=
 =?utf-8?B?VHNlZnhmV203RzA1Z0cxbE1DTWFzS0xBdTJJVWtYVktMQ0FPQ1ZwMGorUDZi?=
 =?utf-8?B?QVVsZXBER0dPdFRvSXB1NlBLcnZ4c0kxaTNmOUtSbmpib1VKbEVSMDdZR1J4?=
 =?utf-8?B?b1RCcU1FaEVRQTk5MUpEdVN2SUs2T1FXYW10S3dUNHo0bmRtaGFFajBtN2px?=
 =?utf-8?B?cjZndXhJTi93NkwwL1NmaXMxYnNoZ21rVlBEVDBOak1ka3JFT2kvYUxvR1h1?=
 =?utf-8?B?aytqZ2o0eFEvNGdhVmg0YlZaY2lkNG0yVUdZZU9mWGV1Q3ExMkJscmRrZkJH?=
 =?utf-8?B?RDBDalNjQ3JJUy9LTzc4L2pQb2F2RDl3WmliUk5mL01wTGNjbDE1MlJsNCsx?=
 =?utf-8?B?T0ZwUWNxYlBjWDlBbmZoZnpDSDF3WUlsbDI4QzVscEdxQVVBUlIyVjIraWR3?=
 =?utf-8?B?VDV4dW9VNXNBaXpQd0VKTytFdVZrOE4xWG1uZ0g3SG12SFM5RVhVc1g0S25K?=
 =?utf-8?B?cWdVMjV2ejVvMkRlWDlodi9Vei9NSlp5Q0RhZ2RRVnQrWEZUODU2OUtvZnUr?=
 =?utf-8?B?VGFtc2NtbHc5Zk1HZGJpWDRMQ0hWb3ZCc2tuemhSSmRKWDgxWm51QUhsaFFS?=
 =?utf-8?B?dVZCOVVNbWZMOTJJZ1ZWSS9XQkpUNms5dGRYbWVjNE1FeWJpTnFIWlJCaHhy?=
 =?utf-8?B?WDYxdWtLb2tBcUEybkp3UnFNVDkzZVNibnEwS1N4KzFmeU9CVG40NWtUeGtH?=
 =?utf-8?B?M01Zemp3bThQQS8zUkFoNFBIN1BLOHQ5eHB5MHVSTDRDY01adHpHSUZFT05W?=
 =?utf-8?B?bmkydVZGTGdQQ2ZmcEZHdVZBT2o2QkhwdmhDbmJDbVB6d3dDSXkvWDE2MG5H?=
 =?utf-8?B?RnpCanFKd3lzRi9kUERNWVRvTko1YlV2TUpjZGU5ZFBkazY3NEhtTGRhS2hD?=
 =?utf-8?B?dHc9PQ==?=
X-MS-Exchange-CrossTenant-Network-Message-Id: 084b59c2-dae8-438e-6ffb-08dbd539ade0
X-MS-Exchange-CrossTenant-AuthSource: MW4PR11MB5776.namprd11.prod.outlook.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 25 Oct 2023 09:06:31.3427
 (UTC)
X-MS-Exchange-CrossTenant-FromEntityHeader: Hosted
X-MS-Exchange-CrossTenant-Id: 46c98d88-e344-4ed4-8496-4ed7712e255d
X-MS-Exchange-CrossTenant-MailboxType: HOSTED
X-MS-Exchange-CrossTenant-UserPrincipalName: dEmLX4ooPfLDoeLvrDd7zdvD7IRLnKPpgeERPt6K4BlzAm05dpVgtX6e+mrrfoJjmkNScxD2HFvxEpUSuT7k8VElRw+m0U2mRDF0PS8/bEE=
X-MS-Exchange-Transport-CrossTenantHeadersStamped: PH7PR11MB8569
X-OriginatorOrg: intel.com
X-Spam-Status: No, score=-0.9 required=5.0 tests=DKIMWL_WL_HIGH,DKIM_SIGNED,
	DKIM_VALID,DKIM_VALID_AU,HEADER_FROM_DIFFERENT_DOMAINS,
	MAILING_LIST_MULTI,SPF_HELO_NONE,SPF_PASS autolearn=unavailable
	autolearn_force=no version=3.4.6
X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on morse.vger.email
Precedence: bulk
List-ID: <linux-kernel.vger.kernel.org>
X-Mailing-List: linux-kernel@vger.kernel.org
X-Greylist: Sender passed SPF test, not delayed by milter-greylist-4.6.4 (morse.vger.email [0.0.0.0]); Wed, 25 Oct 2023 02:07:53 -0700 (PDT)


On 25.10.2023 05:27, Dust Li wrote:
> When mlx5_packet_reformat_alloc() fails, the encap_header allocated in
> mlx5e_tc_tun_create_header_ipv4{6} will be released within it. However,
> e->encap_header is already set to the previously freed encap_header
> before mlx5_packet_reformat_alloc(). As a result, the later
> mlx5e_encap_put() will free e->encap_header again, causing a double free
> issue.
> 
> mlx5e_encap_put()
>     --> mlx5e_encap_dealloc()
>         --> kfree(e->encap_header)

nit: I think it should mlx5e_encap_put_locked not mlx5e_encap_put to be precise.

> 
> This happens when cmd: MLX5_CMD_OP_ALLOC_PACKET_REFORMAT_CONTEXT fail.
> 
> This patch fix it by not setting e->encap_header until
> mlx5_packet_reformat_alloc() success.
> 
> Fixes: d589e785baf5e("net/mlx5e: Allow concurrent creation of encap entries")
> Reported-by: Cruz Zhao <cruzzhao@linux.alibaba.com>
> Reported-by: Tianchen Ding <dtcccc@linux.alibaba.com>
> Signed-off-by: Dust Li <dust.li@linux.alibaba.com>
> ---

Reviewed-by: Wojciech Drewek <wojciech.drewek@intel.com>

>  drivers/net/ethernet/mellanox/mlx5/core/en/tc_tun.c | 10 ++++------
>  1 file changed, 4 insertions(+), 6 deletions(-)
> 
> diff --git a/drivers/net/ethernet/mellanox/mlx5/core/en/tc_tun.c b/drivers/net/ethernet/mellanox/mlx5/core/en/tc_tun.c
> index 00a04fdd756f..8bca696b6658 100644
> --- a/drivers/net/ethernet/mellanox/mlx5/core/en/tc_tun.c
> +++ b/drivers/net/ethernet/mellanox/mlx5/core/en/tc_tun.c
> @@ -300,9 +300,6 @@ int mlx5e_tc_tun_create_header_ipv4(struct mlx5e_priv *priv,
>  	if (err)
>  		goto destroy_neigh_entry;
>  
> -	e->encap_size = ipv4_encap_size;
> -	e->encap_header = encap_header;
> -
>  	if (!(nud_state & NUD_VALID)) {
>  		neigh_event_send(attr.n, NULL);
>  		/* the encap entry will be made valid on neigh update event
> @@ -322,6 +319,8 @@ int mlx5e_tc_tun_create_header_ipv4(struct mlx5e_priv *priv,
>  		goto destroy_neigh_entry;
>  	}
>  
> +	e->encap_size = ipv4_encap_size;
> +	e->encap_header = encap_header;
>  	e->flags |= MLX5_ENCAP_ENTRY_VALID;
>  	mlx5e_rep_queue_neigh_stats_work(netdev_priv(attr.out_dev));
>  	mlx5e_route_lookup_ipv4_put(&attr);
> @@ -568,9 +567,6 @@ int mlx5e_tc_tun_create_header_ipv6(struct mlx5e_priv *priv,
>  	if (err)
>  		goto destroy_neigh_entry;
>  
> -	e->encap_size = ipv6_encap_size;
> -	e->encap_header = encap_header;
> -
>  	if (!(nud_state & NUD_VALID)) {
>  		neigh_event_send(attr.n, NULL);
>  		/* the encap entry will be made valid on neigh update event
> @@ -590,6 +586,8 @@ int mlx5e_tc_tun_create_header_ipv6(struct mlx5e_priv *priv,
>  		goto destroy_neigh_entry;
>  	}
>  
> +	e->encap_size = ipv6_encap_size;
> +	e->encap_header = encap_header;
>  	e->flags |= MLX5_ENCAP_ENTRY_VALID;
>  	mlx5e_rep_queue_neigh_stats_work(netdev_priv(attr.out_dev));
>  	mlx5e_route_lookup_ipv6_put(&attr);