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

Aviv Heller aviv at avivh.com
Sun Sep 24 17:05:42 UTC 2017


> 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.

> 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.

Thanks,
Aviv


More information about the Swan-dev mailing list