Re: kern/179264: [vimage] [pf] Core dump with Packet filter and VIMAGE options compile in a kernel

[glebius]

Synopsis: [vimage] [pf] Core dump with Packet filter and VIMAGE options compile 
in a kernel

State-Changed-From-To: open->analyzed
State-Changed-By: glebius
State-Changed-When: Thu Jun 6 14:52:13 UTC 2013
State-Changed-Why: 
There is work in progress in this area:

http://lists.freebsd.org/pipermail/freebsd-virtualization/2013-June/001296.html

http://www.freebsd.org/cgi/query-pr.cgi?pr=179264
___
freebsd-net@freebsd.org mailing list
http://lists.freebsd.org/mailman/listinfo/freebsd-net
To unsubscribe, send any mail to "freebsd-net-unsubscr...@freebsd.org"

IN6_IS_ADDR_* macros use invalid type punning?

[Matthias Andree]

-- NOTE -- Please Cc: me on replies, I am not subscribed to freebsd-net.


Greetings,

I am just staring at gcc 4.8 warnings when compiling the try.c code
shown below:


$ cat try.c
#include 
#include 
#include 
#include 

int f(void) {
struct sockaddr_in6 sin6;

int r = IN6_IS_ADDR_V4MAPPED((&sin6.sin6_addr));
return r;
}



$ gcc48 -Wall -O2 -c try.c
try.c: In function 'f':
try.c:9:5: warning: dereferencing type-punned pointer will break
strict-aliasing rules [-Wstrict-aliasing]
 int r = IN6_IS_ADDR_V4MAPPED((&sin6.sin6_addr));
 ^
try.c:9:5: warning: dereferencing type-punned pointer will break
strict-aliasing rules [-Wstrict-aliasing]
try.c:9:5: warning: dereferencing type-punned pointer will break
strict-aliasing rules [-Wstrict-aliasing]


This is on FreeBSD 9.1-RELEASE amd64.

/usr/include/netinet6/in6.h contains:

/*
 * Mapped
 */
#define IN6_IS_ADDR_V4MAPPED(a)   \
((*(const u_int32_t *)(const void *)(&(a)->s6_addr[0]) == 0) && \
 (*(const u_int32_t *)(const void *)(&(a)->s6_addr[4]) == 0) && \
 (*(const u_int32_t *)(const void *)(&(a)->s6_addr[8]) ==
ntohl(0x)))


So we indeed break C99/C11 aliasing rules here.


Pedantically speaking, it would also have to be htonl() instead of
ntohl() because the in6_addr is in network order, but it does not matter
to the machine because either maps to __bswap32() anyway.




The same in6.h also declares:

struct in6_addr {
union {
uint8_t __u6_addr8[16];
uint16_t__u6_addr16[8];
uint32_t__u6_addr32[4];
} __u6_addr;/* 128-bit IP6 address */
};


Which would appear to open up an alias-safe way  (sanctioned by the C
standard, access through union containing the type) to reimplement the
macro, which also makes the GCC 4.8 warning go away and is far more
readable:


/*
 * Mapped
 */
#define IN6_IS_ADDR_V4MAPPED(a) \
((a)->__u6_addr.__u6_addr32[0] == 0 && \
 (a)->__u6_addr.__u6_addr32[1] == 0 && \
 (a)->__u6_addr.__u6_addr16[2] == htonl(0x))


Similar considerations apply to the other IN6_IS_ADDR_* macros.


Now, what do we do?

Can we get these fixed in a reasonable timeframe?

To whom, or where, would I submit a patch for all the macros so that it
actually gets committed to HEAD and MFC'd to /9 and /8?

Do we have unit tests for these macros?  Should we add some?


Best
Matthias
(I don't have src commit permission.)
-- NOTE -- Please Cc: me on replies, I am not subscribed to freebsd-net.
___
freebsd-net@freebsd.org mailing list
http://lists.freebsd.org/mailman/listinfo/freebsd-net
To unsubscribe, send any mail to "freebsd-net-unsubscr...@freebsd.org"

Re: IN6_IS_ADDR_* macros use invalid type punning?

[Maxim Dounin]

Hello!

On Thu, Jun 06, 2013 at 10:23:30PM +0200, Matthias Andree wrote:

> I am just staring at gcc 4.8 warnings when compiling the try.c code
> shown below:

[...]

> try.c:9:5: warning: dereferencing type-punned pointer will break
> strict-aliasing rules [-Wstrict-aliasing]
>  int r = IN6_IS_ADDR_V4MAPPED((&sin6.sin6_addr));
>  ^
> try.c:9:5: warning: dereferencing type-punned pointer will break
> strict-aliasing rules [-Wstrict-aliasing]

[...]

> Can we get these fixed in a reasonable timeframe?
> 
> To whom, or where, would I submit a patch for all the macros so that it
> actually gets committed to HEAD and MFC'd to /9 and /8?

Gleb already committed a fix for this 16 months ago 
(unfortunately, correct patch description was lost in transit):

http://svnweb.freebsd.org/base?view=revision&revision=230584

Probably it's a good idea to MFC the fix.

-- 
Maxim Dounin
http://nginx.org/en/donation.html
___
freebsd-net@freebsd.org mailing list
http://lists.freebsd.org/mailman/listinfo/freebsd-net
To unsubscribe, send any mail to "freebsd-net-unsubscr...@freebsd.org"

Re: IN6_IS_ADDR_* macros use invalid type punning?

[Matthias Andree]

Am 07.06.2013 01:29, schrieb Maxim Dounin:

> [...]
> 
>> try.c:9:5: warning: dereferencing type-punned pointer will break
>> strict-aliasing rules [-Wstrict-aliasing]
>>  int r = IN6_IS_ADDR_V4MAPPED((&sin6.sin6_addr));
>>  ^
>> try.c:9:5: warning: dereferencing type-punned pointer will break
>> strict-aliasing rules [-Wstrict-aliasing]
> 
> [...]

> Gleb already committed a fix for this 16 months ago 
> (unfortunately, correct patch description was lost in transit):
> 
> http://svnweb.freebsd.org/base?view=revision&revision=230584

Great.  Thank you for the pointer.  I could have checked head/ first
indeed.

Looking at
:
The code committed at that time is lucky that htonl() and ntohl() are
implemented the same way; all changed macros should be changed to use ==
htonl(1) or == htonl(0x), just to get the proper meaning across.

ntohl would have to be applied to the __u6_addr instead, but is less
efficient because it is not open to compile-time evaluation, unlike
htonl(CONSTANT_ADDRESS).


And indeed the commit log is a bit less compelling than might have
fostered the propagation.  It looks like it were only about qualifiers,
but it is also about violating aliasing rules per ISO 9899.

> Probably it's a good idea to MFC the fix.

Please let's get this MFC'd and MFS'd (while it won't make releng/8.4 at
least we can have stable/8 fixed, too) and get the system-headers
induced warning done away with on all supported branches.

Best
Matthias
___
freebsd-net@freebsd.org mailing list
http://lists.freebsd.org/mailman/listinfo/freebsd-net
To unsubscribe, send any mail to "freebsd-net-unsubscr...@freebsd.org"