[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