Hello,

I tested the patch and it works. I can see all partitions inside my disk. I
just cleaned the first typecast as it was useless and I'll send in a
cleaner patch email to this list.

Now what would be the next steps? Is it worth to submit to gcc? Should I
submit this directly to kernel mainstream?

I also looked for usages of this function. The bug will affect any arch
that does not use an cpu instruction for le/be conversion and for any use
of get_unaligned_le32, get_unaligned_le for 32bit or 64bit functions (that
uses 32bit version twice), get_unaligned for 32bit or 64bit functions when
running on little endian systems, and any derivated macro
like SYS_IND(p), flat_get_addr_from_rp, NL_INTEGER, NL_INT64. This list
might not be complete. For .c files, my rough search found these:

./linux-3.3.8/arch/blackfin/kernel/flat.c
./linux-3.3.8/arch/sh/kernel/dwarf.c
./linux-3.3.8/arch/sh/kernel/module.c
./linux-3.3.8/arch/unicore32/boot/compressed/misc.c
./linux-3.3.8/arch/x86/boot/compressed/mkpiggy.c
./linux-3.3.8/arch/x86/boot/tools/build.c
./linux-3.3.8/block/partitions/ldm.c
./linux-3.3.8/block/partitions/msdos.c
./linux-3.3.8/crypto/camellia.c
./linux-3.3.8/drivers/block/aoe/aoecmd.c
./linux-3.3.8/drivers/block/aoe/aoenet.c
./linux-3.3.8/drivers/block/drbd/drbd_nl.c
./linux-3.3.8/drivers/block/osdblk.c
./linux-3.3.8/drivers/bluetooth/hci_bcsp.c
./linux-3.3.8/drivers/char/snsc_event.c

Specially, the non arch part is important.


---
     Luiz Angelo Daros de Luca, Me.
            luizl...@gmail.com


2013/4/28 Sergey Vlasov <v...@altlinux.ru>

> On Sat, Apr 27, 2013 at 08:55:35PM -0300, Luiz Angelo Daros de Luca wrote:
> > I was curious about the "undefined". I found the text:
> >
> >    The integer promotions are performed on each of the operands. The
> >    type of the result is that of the promoted left operand. If the
> >    value of the right operand is negative or is greater than or
> >    equal to the width of the promoted left operand, the behavior is
> >    undefined.
>
> Actually this part is not the problem here - the unsigned char
> argument will be promoted to int before the shift operation, and the
> width of int is 32 on all platforms currently supported by mainline
> Linux, therefore left shift by 24 bits is allowed.  The problem is the
> next paragraph:
>
>    The result of E1 << E2 is E1 left-shifted E2 bit positions; vacated
>    bits are filled with zeros. If E1 has an unsigned type, the value
>    of the result is E1 * 2^E2 , reduced modulo one more than the
>    maximum value representable in the result type. If E1 has a signed
>    type and nonnegative value, and E1 * 2^E2 is representable in the
>    result type, then that is the resulting value; otherwise, the
>    behavior is undefined.
>
> Therefore (0x80 << 24) is undefined, because 0x80000000 is not
> representable as a 32-bit signed int.  Apparently the intent here is
> to outlaw any form of signed integer overflow (note that the behavior
> of right shifts applied to negative signed integer values is not
> "undefined", but "implementation-defined" - probably because this
> operation does not involve overflow).
>
> Note that the older C89 standard did not declare such left shift
> operations as undefined.
>
> GCC 4.8.0 documentation says that it currently does not treat
> overflowing left shifts of signed values as undefined:
>
>   http://gcc.gnu.org/onlinedocs/gcc-4.8.0/gcc/Integers-implementation.html
>
> However, some prerelease versions of gcc-4.8.0 had a bug related to
> such shift operations, which was fixed before the 4.8.0 release:
>
>   http://gcc.gnu.org/bugzilla/show_bug.cgi?id=54027
>
> In particular, the comment 2 there says that the intent of GCC
> developers is to keep such shifts working.
>
> > So, as left operand is 8bit, the right operant must not be over 8. Does a
> > simple typecast solves it?
> >
> > return (u32)((u32)p[0] | (u32)p[1] << 8 | (u32)p[2] << 16 | (u32)p[3] <<
> > 24);
>
> The outer typecast is not required at all (it will be performed during
> return anyway), and the only really required typecast according to the
> standard is the last one (other operations can be performed with
> signed int values, and the "usual arithmetic conversions" of signed
> int and unsigned int will result in unsigned int).
>
> _______________________________________________
> openwrt-devel mailing list
> openwrt-devel@lists.openwrt.org
> https://lists.openwrt.org/mailman/listinfo/openwrt-devel
>
>
_______________________________________________
openwrt-devel mailing list
openwrt-devel@lists.openwrt.org
https://lists.openwrt.org/mailman/listinfo/openwrt-devel

Reply via email to