[Swan-dev] [PATCH libreswan v2] netlink: Silence negative shift coverity false warning

[Antony Antony]

Hi Aviv,

While v2 patch makes coverity happy the value of feature_bit will be 
different from what it was before.  

> -     feature_bit = 1 << (netlink_esp_hw_offload % 31);
> +     feature_bit = 1 << ((unsigned int)netlink_esp_hw_offload % 31);


On Thu, Aug 31, 2017 at 02:26:04PM +0000, Aviv Heller wrote:
> Antony, regarding the "negative shift" coverity warning, I wasn't able to 
> detect any bug there, and think it is false alarm. 

I re-red your earlier messages and think it is a false warning.

Then a solution is add a "passert (netlink_esp_hw_offload > 1);" Could you 
test the attached patch with HW? Just want to make sure I didn't break it.

I could have used passeert(netlink_esp_hw_offload > 0), however with this

blocks = 0
cmd->features[blocks-1] will have -ne index. Which is probably not good.

Please, let me know if the patch works.

-antony


On Mon, Sep 04, 2017 at 05:15:46PM +0300, avivh at mellanox.com wrote:
> From: Aviv Heller <avivh at mellanox.com>
> 
> Cast to unsigned before shift.
> 
> Signed-off-by: Aviv Heller <avivh at mellanox.com>
> ---
> v1 -> v2:
>     - Cast to unsigned instead of changing the type and values.
> ---
>  programs/pluto/kernel_netlink.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/programs/pluto/kernel_netlink.c b/programs/pluto/kernel_netlink.c
> index 0ed4e31..3c07b52 100644
> --- a/programs/pluto/kernel_netlink.c
> +++ b/programs/pluto/kernel_netlink.c
> @@ -919,7 +919,7 @@ static enum iface_nic_offload netlink_detect_offload(const char *ifname)
>  
>  	/* Feature is supported by kernel. Query device features */
>  	blocks = (netlink_esp_hw_offload + 31) / 32;
> -	feature_bit = 1 << (netlink_esp_hw_offload % 31);
> +	feature_bit = 1 << ((unsigned int)netlink_esp_hw_offload % 31);
>  
>  	cmd = alloc_bytes(sizeof(*cmd) + sizeof(cmd->features[0]) * blocks, "ethtool_gfeatures");
>  	jam_str(ifr.ifr_name, sizeof(ifr.ifr_name), ifname);
> -- 
> 1.8.3.1
> 
> _______________________________________________
> Swan-dev mailing list
> Swan-dev at lists.libreswan.org
> https://lists.libreswan.org/mailman/listinfo/swan-dev
-------------- next part --------------
>From 39744f56220ba3da93f283251b7c2dc6dd5ddf8a Mon Sep 17 00:00:00 2001
From: Antony Antony <antony at phenome.org>
Date: Thu, 14 Sep 2017 11:35:14 +0000
Subject: [PATCH] pluto: passert(netlink_esp_hw_offload > 0) coverity warning

---
 programs/pluto/kernel_netlink.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/programs/pluto/kernel_netlink.c b/programs/pluto/kernel_netlink.c
index 76f1b5e..8f664cd 100644
--- a/programs/pluto/kernel_netlink.c
+++ b/programs/pluto/kernel_netlink.c
@@ -917,6 +917,8 @@ static enum iface_nic_offload netlink_detect_offload(const char *ifname)
 	if (netlink_esp_hw_offload == NIC_OFFLOAD_UNSUPPORTED)
 		return ret;

+	passert(netlink_esp_hw_offload > 1);
+
 	/* Feature is supported by kernel. Query device features */
 	blocks = (netlink_esp_hw_offload + 31) / 32;
 	feature_bit = 1 << (netlink_esp_hw_offload % 31);
--
2.9.5