netdev - Re: sparse vs. skbuff.h

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <4B01B94C.4050401@gmail.com>
Date:	Mon, 16 Nov 2009 21:42:52 +0100
From:	Eric Dumazet <eric.dumazet@...il.com>
To:	Vegard Nossum <vegard.nossum@...il.com>
CC:	Johannes Berg <johannes@...solutions.net>,
	netdev <netdev@...r.kernel.org>
Subject: Re: sparse vs. skbuff.h

Vegard Nossum a écrit :
> 2009/11/16 Eric Dumazet <eric.dumazet@...il.com>:
>> Johannes Berg a écrit :
>>> On Mon, 2009-11-16 at 20:21 +0100, Johannes Berg wrote:
>>>> commit 14d18a81b5171d4433e41129619c75748b4f4d26
>>>> Author: Eric Dumazet <eric.dumazet@...il.com>
>>>> Date:   Thu Oct 29 00:10:37 2009 +0000
>>>>
>>>>     net: fix kmemcheck annotations
>>>>
>>>>
>>>> broke sparse endian checks on everything that includes skbuff.h because
>>>> the first and only (because it's an error) thing sparse now reports is
>>>> this:
>>>>
>>>> include/linux/skbuff.h:357:41: error: invalid bitfield specifier for type restricted __be16.
>>> Simply changing from
>>>       __be16 protocol:16;
>>> to
>>>       __be16 protocol;
>>>
>>> but leaving it inside the kmemcheck annotation seems to do the right
>>> thing. Except of course that kmemcheck will not properly check it now.
>>> Maybe those annotations should simply be made to have no impact on
>>> struct padding instead?
>>>
>> Hmm, I have really no idea of what is the right way to fix this stuff.
>>
>> Last time I did adding a non bitfield element inside the begin/end annotations,
>> I was flamed, because a bitfield is a bitfield, not a char/short
>>
>> http://www.spinics.net/lists/netdev/msg108803.html
>>
>> Now sparse is complaining... What will be the next story ?
> 
> If by "I was flamed" you are referring to my reply:
> 
> http://www.spinics.net/lists/netdev/msg108825.html
> 
> then I am really sorry, because I had no intentions to insult you. In
> fact, I am grateful that you are finding bugs and telling me about
> them But I should also be allowed to disagree with a patch if I truly
> believe it is the wrong thing to do.

Oh well, maybe flamed is not the right word Vegard, and I did not feel
being insulted at all ! Sorry if the tone of my mail was misleading.

> 
> For the issue in question: If the variable is turned into a
> non-bitfield (as Johannes suggested), it would be fine, because now
> GCC won't emit masking operations (AND, OR) when initializing it, but
> a regular MOV. Also, the struct annotations do not by themselves do
> anything, but they are used by kmemcheck_annotate_bitfield().
> 
> In other words, I think the right thing to do is to turn it into a
> non-bitfield and move it _outside_ the bitfield annotation. Johannes,
> can you make the patch and let us have a look? In the meantime I will
> submit the patch that fixes the extraneous field padding in
> KMEMCHECK=n kernels.
> 

Thanks
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html