netdev - Re: [bug report] net/mlx5: Add direct rule fs

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

Message-ID: <YozXHxO85GpT+5GK@unreal>
Date:   Tue, 24 May 2022 16:01:19 +0300
From:   Leon Romanovsky <leon@...nel.org>
To:     Dan Carpenter <dan.carpenter@...cle.com>
Cc:     maorg@...dia.com, linux-rdma@...r.kernel.org,
        Saeed Mahameed <saeedm@...dia.com>,
        linux-netdev <netdev@...r.kernel.org>
Subject: Re: [bug report] net/mlx5: Add direct rule fs_cmd implementation

On Wed, May 11, 2022 at 04:57:24PM +0300, Dan Carpenter wrote:
> [ I was reviewing old use after free warnings and stumbled across this
>   one which still needs fixing - dan ]
> 
> Hello Maor Gottlieb,
> 
> The patch 6a48faeeca10: "net/mlx5: Add direct rule fs_cmd
> implementation" from Aug 20, 2019, leads to the following Smatch
> static checker warning:
> 
> 	drivers/net/ethernet/mellanox/mlx5/core/steering/fs_dr.c:53 set_miss_action()
> 	warn: 'action' was already freed.
> 
> drivers/net/ethernet/mellanox/mlx5/core/steering/fs_dr.c
>     28 static int set_miss_action(struct mlx5_flow_root_namespace *ns,
>     29                            struct mlx5_flow_table *ft,
>     30                            struct mlx5_flow_table *next_ft)
>     31 {
>     32         struct mlx5dr_action *old_miss_action;
>     33         struct mlx5dr_action *action = NULL;
>     34         struct mlx5dr_table *next_tbl;
>     35         int err;
>     36 
>     37         next_tbl = next_ft ? next_ft->fs_dr_table.dr_table : NULL;
>     38         if (next_tbl) {
>     39                 action = mlx5dr_action_create_dest_table(next_tbl);
>     40                 if (!action)
>     41                         return -EINVAL;
>     42         }
>     43         old_miss_action = ft->fs_dr_table.miss_action;
>     44         err = mlx5dr_table_set_miss_action(ft->fs_dr_table.dr_table, action);
>     45         if (err && action) {
>     46                 err = mlx5dr_action_destroy(action);
>     47                 if (err) {
>     48                         action = NULL;
>     49                         mlx5_core_err(ns->dev, "Failed to destroy action (%d)\n",
>     50                                       err);
>     51                 }
> 
> If "err" is zero then "action" is freed.
> 
>     52         }
> --> 53         ft->fs_dr_table.miss_action = action;
>                                              ^^^^^^
> Use after free.

Thanks for the report.
https://lore.kernel.org/netdev/7fe70bbb120422cc71e6b018531954d58ea2e61e.1653397057.git.leonro@nvidia.com/T/#u

> 
>     54         if (old_miss_action) {
>     55                 err = mlx5dr_action_destroy(old_miss_action);
>     56                 if (err)
>     57                         mlx5_core_err(ns->dev, "Failed to destroy action (%d)\n",
>     58                                       err);
>     59         }
>     60 
>     61         return err;
>     62 }
> 
> regards,
> dan carpenter