netdev - re: net/mlx5: IPsec, Refactor SA handle creation and destruction

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

Date:   Tue, 21 Apr 2020 14:59:56 +0100
From:   Colin Ian King <colin.king@...onical.com>
To:     Raed Salem <raeds@...lanox.com>
Cc:     Boris Pismenny <borisp@...lanox.com>,
        Saeed Mahameed <saeedm@...lanox.com>,
        Leon Romanovsky <leon@...nel.org>,
        Tariq Toukan <tariqt@...lanox.com>,
        "netdev@...r.kernel.org" <netdev@...r.kernel.org>,
        linux-rdma@...r.kernel.org,
        "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>
Subject: re: net/mlx5: IPsec, Refactor SA handle creation and destruction

Hi,

Static analysis with Coverity has detected a potential issue with the
following commit:

commit 7dfee4b1d79e1800818abcfb47747b162c9a2d31
Author: Raed Salem <raeds@...lanox.com>
Date:   Wed Oct 23 17:04:13 2019 +0300

    net/mlx5: IPsec, Refactor SA handle creation and destruction

The issue is in mlx5_fpga_is_ipsec_device() in
drivers/net/ethernet/mellanox/mlx5/core/fpga/ipsec.c as follows:


710  Bitwise-and with zero
     bit_and_with_zero: accel_xfrm->attrs.action &
MLX5_ACCEL_ESP_ACTION_DECRYPT is always 0.  This occurs as the logical
operand of if.

711        if (accel_xfrm->attrs.action & MLX5_ACCEL_ESP_ACTION_DECRYPT) {
Logically dead code (DEADCODE)

712                err = ida_simple_get(&fipsec->halloc, 1, 0, GFP_KERNEL);
713                if (err < 0) {
714                        context = ERR_PTR(err);
715                        goto exists;
716                }
717
718                sa_ctx->sa_handle = err;
719                if (sa_handle)
720                        *sa_handle = sa_ctx->sa_handle;
721        }

in include/linux/mlx5/accel.h MLX5_ACCEL_ESP_ACTION_DECRYPT is defined
as zero:

50 enum mlx5_accel_esp_action {
51        MLX5_ACCEL_ESP_ACTION_DECRYPT,
52        MLX5_ACCEL_ESP_ACTION_ENCRYPT,
53 };


I believe there are some other instances of this bit-wise and-ing with
zero, e.g. in mlx5_fpga_ipsec_release_sa_ctx() we have:

855     if (sa_ctx->fpga_xfrm->accel_xfrm.attrs.action &
856         MLX5_ACCEL_ESP_ACTION_DECRYPT)

Colin