linux-kernel - Re: [PATCH 08/17] binfmt_flat: consolidate two version of flat_v2_reloc

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] [thread-next>] [day] [month] [year] [list]

Date:   Wed, 26 Jun 2019 17:23:11 +1000
From:   Greg Ungerer <gerg@...ux-m68k.org>
To:     Al Viro <viro@...iv.linux.org.uk>, Christoph Hellwig <hch@....de>
Cc:     Michal Simek <monstr@...str.eu>,
        linux-arm-kernel@...ts.infradead.org, linux-c6x-dev@...ux-c6x.org,
        uclinux-h8-devel@...ts.sourceforge.jp,
        linux-m68k@...ts.linux-m68k.org, linux-riscv@...ts.infradead.org,
        linux-sh@...r.kernel.org, linux-xtensa@...ux-xtensa.org,
        linux-kernel@...r.kernel.org
Subject: Re: [PATCH 08/17] binfmt_flat: consolidate two version of
 flat_v2_reloc_t


On 26/6/19 8:29 am, Al Viro wrote:
> On Thu, Jun 13, 2019 at 09:08:54AM +0200, Christoph Hellwig wrote:
>> Two branches of the ifdef maze actually have the same content, so merge
>> them.
>>
>> Signed-off-by: Christoph Hellwig <hch@....de>
>> ---
>>   include/linux/flat.h | 6 ++----
>>   1 file changed, 2 insertions(+), 4 deletions(-)
>>
>> diff --git a/include/linux/flat.h b/include/linux/flat.h
>> index 2b7cda6e9c1b..19c586b74b99 100644
>> --- a/include/linux/flat.h
>> +++ b/include/linux/flat.h
>> @@ -69,15 +69,13 @@ struct flat_hdr {
>>   typedef union {
>>   	unsigned long	value;
>>   	struct {
>> -# if defined(mc68000) && !defined(CONFIG_COLDFIRE)
>> +#if defined(__LITTLE_ENDIAN_BITFIELD) || \
>> +    (defined(mc68000) && !defined(CONFIG_COLDFIRE))
>>   		signed long offset : 30;
>>   		unsigned long type : 2;
>>   # elif defined(__BIG_ENDIAN_BITFIELD)
>>   		unsigned long type : 2;
>>   		signed long offset : 30;
>> -# elif defined(__LITTLE_ENDIAN_BITFIELD)
>> -		signed long offset : 30;
>> -		unsigned long type : 2;
>>   # else
>>   #   	error "Unknown bitfield order for flat files."
>>   # endif
>> -- 
>> 2.20.1
>>
> 
> FWIW, I wonder if keeping that type is worth bothering.
> Something like
> old_reloc(__be32 reloc)
> {
> 	u32 v = be32_to_cpu(reloc);
> 	int offset, type;
> 
> #if (defined(mc68000) && !defined(CONFIG_COLDFIRE))
> 	/* old m68k uses unusual format - type is in lower bits of octet 3 */
> 	type = v % 4;
> 	offset = (int)v / 4;
> #else
> 	/* everything else (including coldfire) has it in upper bits of octet 0 */
> 	type = v >> 30;
> 	offset = (int)(v << 2) >> 2; /* or (v & 0x1fffffff) - (v & 0x20000000) * 4 */
> #endif
> 	...
> 
> and to hell with bitfields, aliasing unions, etc.  Unless I'm misreading
> the whole thing, that is...  Greg?

I think you are right. This is much better.
The old mc6800 is the odd one out, the rest have it in network order,
and this makes that much clearer.

Regards
Greg