linux-kernel - Re: OpenRISC: Module loader broken

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:	Fri, 2 Mar 2012 07:49:48 +0100
From:	Geert Uytterhoeven <geert@...ux-m68k.org>
To:	Jonas Bonn <jonas@...thpole.se>
Cc:	Richard Weinberger <richard@....at>, linux@...ts.openrisc.net,
	"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>
Subject: Re: OpenRISC: Module loader broken

On Fri, Mar 2, 2012 at 07:11, Jonas Bonn <jonas@...thpole.se> wrote:
>> It explodes somewhere in apply_relocate_add().
>> This function is anyway fishy:
>>                 case R_OR32_CONST:
>>                         location = (uint16_t *)location + 1;
>>                         *((uint16_t *)location) = (uint16_t) (value);
>>                         break;
>>                 case R_OR32_CONSTH:
>>                         location = (uint16_t *)location + 1;
>>                         *((uint16_t *)location) = (uint16_t) (value >> 16);
>>                         break;
>>
>> Why the cast to (uint16_t *)?
>
> This is for loading a 32 bit const, which is done in two steps:
>
> l.movhi rX,hi(const)
> l.ori   rX,rX,lo(const)
>
> The immediate value 'const' here is encoded in the low 16 bits of those
> instructions.

So OpenRISC is big endian, which matches
arch/openrisc/include/asm/byteorder.h:#include <linux/byteorder/big_endian.h>

However, there are some remainings of little endian support:
arch/openrisc/include/asm/unaligned.h:#if defined(__LITTLE_ENDIAN)
arch/openrisc/include/asm/unaligned.h:#elif defined(__BIG_ENDIAN)
arch/openrisc/include/asm/unaligned.h:# error need to define endianess

> Since 'location' points at the 32-bit instruction in question, that cast
> is supposed to be saying:  'move 16 bit value into low 16 bits of
> instruction'.

It may be more readable and safer (w.r.t. changing endianness) to do
32-bit loads and stores and explicitly modify the 16 LSB.

Gr{oetje,eeting}s,

                        Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@...ux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/