| Message ID | 3d1dadb911fa4ebacf1ca5ad298a5c696ed261a6.1461607119.git.marcelo.leitner@gmail.com |
|---|---|
| State | Accepted, archived |
| Delegated to: | David Miller |
| Headers | show |
From: Marcelo Ricardo Leitner <marcelo.leitner@gmail.com> Date: Mon, 25 Apr 2016 15:13:17 -0300 > Fix casting in net_gso_ok. Otherwise the shift on > gso_type << NETIF_F_GSO_SHIFT may hit the 32th bit and make it look like > a INT_MIN, which is then promoted from signed to uint64 which is > 0xffffffff80000000, resulting in wrong behavior when it is and'ed with > the feature itself, as in: ... > So that this: > return (features & feature) == feature; > Actually works on more bits than expected and invalid ones. > > Fix is to promote it earlier. > > Issue noted while rebasing SCTP GSO patch but posting separetely as > someone else may experience this meanwhile. > > Signed-off-by: Marcelo Ricardo Leitner <marcelo.leitner@gmail.com> Great catch, applied, thanks Marcelo.
diff --git a/include/linux/netdevice.h b/include/linux/netdevice.h index 1f6d5db471a24f7f23bbe425c359d64adb5dfd67..13ee05f71e3d74c9d2feef6c371547a4b2f82879 100644 --- a/include/linux/netdevice.h +++ b/include/linux/netdevice.h @@ -3991,7 +3991,7 @@ netdev_features_t netif_skb_features(struct sk_buff *skb); static inline bool net_gso_ok(netdev_features_t features, int gso_type) { - netdev_features_t feature = gso_type << NETIF_F_GSO_SHIFT; + netdev_features_t feature = (netdev_features_t)gso_type << NETIF_F_GSO_SHIFT; /* check flags correspondence */ BUILD_BUG_ON(SKB_GSO_TCPV4 != (NETIF_F_TSO >> NETIF_F_GSO_SHIFT));
Fix casting in net_gso_ok. Otherwise the shift on gso_type << NETIF_F_GSO_SHIFT may hit the 32th bit and make it look like a INT_MIN, which is then promoted from signed to uint64 which is 0xffffffff80000000, resulting in wrong behavior when it is and'ed with the feature itself, as in: This test app: #include <stdio.h> #include <stdint.h> int main(int argc, char **argv) { uint64_t feature1; uint64_t feature2; int gso_type = 1 << 15; feature1 = gso_type << 16; feature2 = (uint64_t)gso_type << 16; printf("%lx %lx\n", feature1, feature2); return 0; } Gives: ffffffff80000000 80000000 So that this: return (features & feature) == feature; Actually works on more bits than expected and invalid ones. Fix is to promote it earlier. Issue noted while rebasing SCTP GSO patch but posting separetely as someone else may experience this meanwhile. Signed-off-by: Marcelo Ricardo Leitner <marcelo.leitner@gmail.com> --- Dave I couldn't find a good way to indent this that wouldn't be uglier than a bit long line, but let me know if you prefer otherwise. include/linux/netdevice.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)