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

[Paul Wouters]

I was also waiting on this patch to ensure it goes into RHEL. 

Sent from my iPhone

> On Oct 4, 2017, at 14:14, Antony Antony <antony at phenome.org> wrote:
> 
> Hi Aviv,
> 
> On Sun, Sep 24, 2017 at 05:05:42PM +0000, Aviv Heller wrote:
>>> coverity-detected anomalies are sometimes subtle.  So I looked at this
>>> code and found a couple of bugs.  I also did some tidying.  But no
>>> testing!
>>> 
>>> Aviv, Antony: please have a look at commit
>>> f7aaa80198851ef71da4be8dc54e816f7064e29a
>> 
>> Hi Hugh, Antony,
>> 
>> Hugh, thanks for bringing this up, there are indeed a few bugs in the code.
>> 
>>> Note: % 31 is almost never correct.  I think that in this code, + 31 is 
>>> also wrong.
>> 
>> The (% 31) is indeed incorrect and should be (% 32).
>> Also, another bug fixed in your patch is the strneq() being compared to 0 instead of 1.
>> 
>> Digging a little into ethtool ioctl code, the size passed (and correspondingly the array length) in cmd->size to ETHTOOL_GFEATURES should be the total number of blocks (dwords) possible for all options, or basically, (sset_len + 31) / 32 [put in the numbers and see that this calculation is correct].
>> Failing to satisfy this will result in failure in case our ESP bit is no longer located in the last dword.
>> 
>>> Could someone with real hardware test this code?
>> It works, with the limitation described in '2' above.
> 
> is nic-offload=auto broken now in master? 
> 
> If + 31 is correct f7aaa80198851 need fixing? 
> 
> -       unsigned block = netlink_esp_hw_offload / 32;
> +       unsigned block = (netlink_esp_hw_offload + 31) / 32;
> 
>>> I cannot see that the old code would use NIC offload since the test for 
>>> the feature seemed to be backwards.
>>> 
>>> Of course I could be backwards.
>> No, it indeed didn't work.
>> 
>> I also created a patch with all these fixes based on the original code, in case it is needed.
> 
> Did you send a patch? I do not have anything else other than the old one,
> [Swan-dev] [PATCH libreswan v2], Sep 04, 2017
> 
> regards,
> -antony
> _______________________________________________
> Swan-dev mailing list
> Swan-dev at lists.libreswan.org
> https://lists.libreswan.org/mailman/listinfo/swan-dev