Skip to contentSkip to conversation

vxlan: keep flags and vni in network byte order

bug report Dan Carpenter 2 participants 4 messages Nov 11, 2016 Kernel janitors

lore
Share & tools
lore
#0 lore raw

Hello Jiri Benc,

The patch 54bfd872bf16: "vxlan: keep flags and vni in network byte order" from Feb 16, 2016, leads to the following static checker warning:

	./include/net/vxlan.h:340 vxlan_vni()
	warn: potential shift truncation.  '0xffffff00 << 8'

include/net/vxlan.h

   335  static inline __be32 vxlan_vni(__be32 vni_field)
   336  {
   337  #if defined(__BIG_ENDIAN)

338 return (__force __be32)((__force u32)vni_field >> 8);
339 #else
340 return (__force __be32)((__force u32)(vni_field & VXLAN_VNI_MASK) << 8);

Are you sure this is correct? VXLAN_VNI_MASK already has an << 8 shift so this feels like maybe it is a double shift.

341 #endif
342 }

regards,
dan carpenter

#1 lore raw
Quoted 17 lines · Dan Carpenter

Hello Jiri Benc,

The patch 54bfd872bf16: "vxlan: keep flags and vni in network byte order" from Feb 16, 2016, leads to the following static checker warning:

	./include/net/vxlan.h:340 vxlan_vni()
	warn: potential shift truncation.  '0xffffff00 << 8'

include/net/vxlan.h

   335  static inline __be32 vxlan_vni(__be32 vni_field)
   336  {
   337  #if defined(__BIG_ENDIAN)
   338          return (__force __be32)((__force u32)vni_field >> 8);
   339  #else
   340          return (__force __be32)((__force u32)(vni_field & VXLAN_VNI_MASK) << 8);
Quoted 2 lines

Are you sure this is correct? VXLAN_VNI_MASK already has an << 8 shift so this feels like maybe it is a double shift.

The static checker clearly can't understand endian conversion macros. VXLAN_VNI_MASK is:

cpu_to_be32(VXLAN_VID_MASK << 8)

Thus on little endian, it becomes 0x00ffffff.

Jiri

#2 lore raw
In reply to Dan Carpenter
Quoted 6 lines · Jiri Benc

The static checker clearly can't understand endian conversion macros. VXLAN_VNI_MASK is:

cpu_to_be32(VXLAN_VID_MASK << 8)

Thus on little endian, it becomes 0x00ffffff.

Btw, until the unspecified static checker you used is not fixed to resolve such macros correctly, it can't be really used to check the kernel source, yet alone to generate bug reports. It will trip over false positive in gazillion places.

Jiri

#3 lore raw
In reply to Dan Carpenter
Quoted 7 lines · Jiri Benc
Quoted 6 lines · Jiri Benc

The static checker clearly can't understand endian conversion macros. VXLAN_VNI_MASK is:

cpu_to_be32(VXLAN_VID_MASK << 8)

Thus on little endian, it becomes 0x00ffffff.

Ah right.

Quoted 5 lines

Btw, until the unspecified static checker you used is not fixed to resolve such macros correctly, it can't be really used to check the kernel source, yet alone to generate bug reports. It will trip over false positive in gazillion places.

Nope. Just this one.

regards,
dan carpenter