netdev - Re: [-next] openvswitch BUILD_BUG

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]

Date:	Wed, 4 Sep 2013 10:53:29 -0700
From:	Jesse Gross <jesse@...ira.com>
To:	Geert Uytterhoeven <geert@...ux-m68k.org>
Cc:	David Miller <davem@...emloft.net>, Andy Zhou <azhou@...ira.com>,
	"dev@...nvswitch.org" <dev@...nvswitch.org>,
	netdev <netdev@...r.kernel.org>,
	Linux Kernel Mailing List <linux-kernel@...r.kernel.org>,
	Linux-Next <linux-next@...r.kernel.org>
Subject: Re: [-next] openvswitch BUILD_BUG_ON failed

On Tue, Sep 3, 2013 at 11:55 PM, Geert Uytterhoeven
<geert@...ux-m68k.org> wrote:
> On Tue, Sep 3, 2013 at 11:44 PM, Jesse Gross <jesse@...ira.com> wrote:
>> On Sat, Aug 31, 2013 at 5:11 AM, Geert Uytterhoeven
>> <geert@...ux-m68k.org> wrote:
>>> On Fri, Aug 30, 2013 at 3:11 AM, Jesse Gross <jesse@...ira.com> wrote:
>>>> On Thu, Aug 29, 2013 at 3:10 PM, David Miller <davem@...emloft.net> wrote:
>>>>> From: Jesse Gross <jesse@...ira.com>
>>>>> Date: Thu, 29 Aug 2013 14:42:22 -0700
>>>>>
>>>>>> On Thu, Aug 29, 2013 at 2:21 PM, Geert Uytterhoeven
>>>>>> <geert@...ux-m68k.org> wrote:
>>>>>>> However, I have some doubts about other alignment "enforcements":
>>>>>>>
>>>>>>> "__aligned(__alignof__(long))" makes the whole struct aligned to the
>>>>>>> alignment rule for "long":
>>>>>>>    1. This is only 2 bytes on m68k, i.e. != sizeof(long).
>>>>>>>    2. This is 4 bytes on many 32-bit platforms, which may be less than the
>>>>>>>       default alignment for "__be64" (cfr. some members of struct
>>>>>>>       ovs_key_ipv4_tunnel), so this may make those 64-bit members unaligned.
>>>>>>
>>>>>> Do any of those 32-bit architectures actually care about alignment of
>>>>>> 64 bit values? On 32-bit x86, a long is 32 bits but the alignment
>>>>>> requirement of __be64 is also 32 bit.
>>>>>
>>>>> All except x86-32 do, it is in fact the odd man out with respect to this
>>>>> issue.
>>>>
>>>> Thanks, good to know.
>>>>
>>>> Andy, do you want to modify your patch to just drop the alignment
>>>> specification as Geert suggested (but definitely keep the new build
>>>> assert that you added)? It's probably better to just send the patch to
>>>> netdev (against net-next) as well since you'll likely get better
>>>> comments there and we can fix this faster if you cut out the
>>>> middleman.
>>>
>>> Why do you want to keep the build asserts?
>>> Is this in-memory structure also transfered as-is over the network?
>>> If yes, you definitely want the padding.
>>
>> Well they caught this bug and really don't cost anything.
>>
>>> Nevertheless, as the struct contains u32 and even __be64 members, the
>>> size of the struct will always be a multiple of the alignment unit for
>>> 64-bit quantities (and thus also for long), as per the C standard.
>>> Hence the check
>>>
>>>     BUILD_BUG_ON(sizeof(struct sw_flow_key) % __alignof__(long));
>>>
>>> will only catch bad compiler bugs or people adding __packed to the struct.
>>
>> It's possible that we might want to pack the structure in the future.
>> More generally though, the contents of the struct is really
>> independent of the alignment requirements here because we're accessing
>> it as an array of bytes in long-sized chunks so implicitly depending
>> on the size of the members is not that great.
>
> So you're accessing it as an array of bytes in long-sized chunks.
> What are you doing with this accessed data?
> Transfering over the network?
> Storing on disk?
> Then it must be portable across machines and architectures, right?

It's just an in-memory hash table lookup. No one else ever sees it.
--
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