linux-kernel - RE: [EXT] Re: [net PATCH] octeontx2-af: Fix marking couple of structure as _

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, 20 Dec 2023 13:04:47 +0000
From: Suman Ghosh <sumang@...vell.com>
To: Jacob Keller <jacob.e.keller@...el.com>,
        David Laight
	<David.Laight@...LAB.COM>,
        "davem@...emloft.net" <davem@...emloft.net>,
        "edumazet@...gle.com" <edumazet@...gle.com>,
        "kuba@...nel.org"
	<kuba@...nel.org>,
        "pabeni@...hat.com" <pabeni@...hat.com>,
        Sunil Kovvuri
 Goutham <sgoutham@...vell.com>,
        Subbaraya Sundeep Bhatta
	<sbhatta@...vell.com>,
        Jerin Jacob Kollanukkaran <jerinj@...vell.com>,
        Geethasowjanya Akula <gakula@...vell.com>,
        Hariprasad Kelam
	<hkelam@...vell.com>,
        Linu Cherian <lcherian@...vell.com>,
        "netdev@...r.kernel.org" <netdev@...r.kernel.org>,
        "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>
Subject: RE: [EXT] Re: [net PATCH] octeontx2-af: Fix marking couple of
 structure as __packed

>>>
>>> However that may not be true across all compilers etc. Also all the
>>> other structures are __packed. Makes sense.
>>
>> Or not - maybe all the __packed should be removed instead!
>>
>> Unless these structures (or any others) appear in 'messages' which get
>> transferred between systems they really shouldn't be __packed.
>> And a 93 byte 'message' with all those fields seems rather odd.
>>
>> The above breakdown seems to imply everything is 'unsigned char'
>> so the __packed makes no difference.
>>
>> Using __packed requires the compiler generate byte loads/store with
>> shifts (etc) on many architectures and should really be avoided unless
>> it is absolutely needed for binary compatibility.
>>
>> Even then if the problem is a 64bit field that only needs to be 32bit
>> aligned (as is common for some compat32 code) then the 64bit fields
>> should be marked as being 32bit aligned.
>>
>> 	David
>>
>Right. Typically packed is only required when dealing with something
>where the exact binary layout matters (i.e. copying to/from hardware or
>across systems in such a way that the layout might change with different
>compilers/arch).
>
>If that isn't how this structure is used, then ya, removing __packed
>seems reasonable. And at least for one system I see no difference in the
>actual generated layout, making __packed redundant.
>
>However, its not clear to me at a glance how this structure is used and
>whether it really is copied between places where binary compatibility is
>a requirement.
>
>> -
>> Registered Address Lakeside, Bramley Road, Mount Farm, Milton Keynes,
>> MK1 1PT, UK Registration No: 1397386 (Wales)
[Suman] Yes, these structures are copied from firmware. It is needed to inform kernel about some parsing information required by hardware. That is the reason structures are packed and these two were missed.