Subject: kern/32645: wrong expression in VLAN_ATTACHED macro (if_ether.h)
To: None <kern-bug-people@netbsd.org, gnats-admin@netbsd.org,>
From: Pavel Cahyna <pcah8322@artax.karlin.mff.cuni.cz>
List: netbsd-bugs
Date: 01/26/2006 23:25:00
>Number:         32645
>Category:       kern
>Synopsis:       wrong expression in VLAN_ATTACHED macro (if_ether.h)
>Confidential:   no
>Severity:       non-critical
>Priority:       medium
>Responsible:    kern-bug-people
>State:          open
>Class:          sw-bug
>Submitter-Id:   net
>Arrival-Date:   Thu Jan 26 23:25:00 +0000 2006
>Originator:     Pavel Cahyna
>Release:        NetBSD 3.0_RC5
>Organization:
>Environment:
System: NetBSD beta 3.0_RC5 NetBSD 3.0_RC5 (EV56) #3: Mon Dec 12 20:28:20 CET 2005 pavel@beta:/usr/src/sys/arch/alpha/compile/EV56 alpha
Architecture: alpha
Machine: alpha
>Description:
if_ether.h has

/* test if any VLAN is configured for this interface */
#define VLAN_ATTACHED(ec)       (&(ec)->ec_nvlans > 0)

-> has higher priority than &, so this code takes the address of
ec->ec_nvlans and compares it against a NULL pointer! I doubt that this is
the itended effect...

The comparison will be always true, so VLAN_OUTPUT_TAG will always try
to find the tag. This probably won't have any other negative effect
than performance degradation in a case without any VLAN.
>How-To-Repeat:
code analysis. Change the 0 to 1, which would be still syntactically
valid for an integer expression and see:

$ make COPTS+=-fsyntax-only if_gsip.o 
#   compile  BETA/if_gsip.o
cc -mno-fp-regs -ffreestanding -g -fsyntax-only -Werror -Wall -Wno-main -Wno-format-zero-length -Wpointer-arith -Wmissing-prototypes -Wstrict-prototypes -Wno-sign-compare -fno-zero-initialized-in-bss -Dalpha -I. -I../../../../arch -I../../../.. -nostdinc -DDIAGNOSTIC -DMSGBUFSIZE=1048576 -DUFS_DIRHASH -DLKM -DAUDIO_DEBUG=5 -DMAXUSERS=32 -D_KERNEL -D_KERNEL_OPT -I../../../../dist/ipf -c ../../../../dev/pci/if_gsip.c
In file included from ../../../../dev/pci/if_gsip.c:43:
../../../../dev/pci/if_sip.c: In function `gsip_start':
../../../../dev/pci/if_sip.c:1366: warning: comparison between pointer and integer
../../../../dev/pci/if_sip.c: In function `gsip_init':
../../../../dev/pci/if_sip.c:2486: warning: comparison between pointer and integer
../../../../dev/pci/if_sip.c:2499: warning: comparison between pointer and integer
../../../../dev/pci/if_sip.c:2508: warning: comparison between pointer and integer
*** Error code 1

>Fix:
--- if_ether.h.~1.38.~	Sun Feb 20 16:41:48 2005
+++ if_ether.h	Fri Jan 27 00:14:38 2006
@@ -273,7 +273,7 @@
 	((*(u_int *)(mtag + 1)) & 4095)
 
 /* test if any VLAN is configured for this interface */
-#define VLAN_ATTACHED(ec)	(&(ec)->ec_nvlans > 0)
+#define VLAN_ATTACHED(ec)	((ec)->ec_nvlans > 0)
 
 void	ether_ifattach(struct ifnet *, const u_int8_t *);
 void	ether_ifdetach(struct ifnet *);