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

[Antony Antony]

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