linux-kernel - Re: [-next] FATAL: drivers/gpu/drm/udl/udl: sizeof(struct usb_device_id)=24 is not a modulo of the size of section __mod_usb_device

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]

Message-ID: <CAMuHMdWobY63O6jSuET1MzuQ80N8xjJA7ny6n0tCdc29Z3vKpA@mail.gmail.com>
Date:	Sat, 16 Jun 2012 21:30:48 +0200
From:	Geert Uytterhoeven <geert@...ux-m68k.org>
To:	Greg Kroah-Hartman <gregkh@...uxfoundation.org>
Cc:	Bjørn Mork <bjorn@...k.no>,
	USB list <linux-usb@...r.kernel.org>,
	linux-kernel@...r.kernel.org,
	Linux-Next <linux-next@...r.kernel.org>,
	linux-kbuild <linux-kbuild@...r.kernel.org>,
	"Linux/m68k" <linux-m68k@...r.kernel.org>
Subject: Re: [-next] FATAL: drivers/gpu/drm/udl/udl: sizeof(struct
 usb_device_id)=24 is not a modulo of the size of section __mod_usb_device_table=44.

On Sat, Jun 16, 2012 at 9:11 PM, Greg Kroah-Hartman
<gregkh@...uxfoundation.org> wrote:
> On Sat, Jun 16, 2012 at 03:23:31PM +0200, Bjørn Mork wrote:
>> Greg Kroah-Hartman <gregkh@...uxfoundation.org> writes:
>> > On Fri, Jun 15, 2012 at 11:02:55PM +0200, Geert Uytterhoeven wrote:
>> >
>> >> As "kernel_ulong_t  driver_info" is no longer naturally aligned, the
>> >> compiler will
>> >> add implicit padding. But the padding depends on the architecture.
>> >
>> > Ah, so we were "lucky" before, nice.
>>
>> I don't really believe in luck :-)  I think someone has been really
>> smart here.  Maybe too smart...
>
> No, I think the previous structure was just "lucky" in that it just
> happened to be the right alignment.  I say this as I think I was the one
> who created that structure years ago.  Or maybe not, this was back in
> the 2.3 kernel days, I can't remember what patches I wrote last week...
>
>> >> It can be fixed by adding explicit padding. Probably it should be padded by
>> >> 7 bytes (not 3), as kernel_ulong_t may require 8-byte alignment on some 64-bit
>> >> platforms. Or by an explicit alignment attribute.
>> >>
>> >> See also
>> >>   * commit 8175fe2dda1c93a9c596921c8ed4a0b4baccdefe ("HID: fix
>> >> hid_device_id for cross compiling")
>> >>   * commit 7492d4a416d68ab4bd254b36ffcc4e0138daa8ff ("sdio: fix module
>> >> device table definition for m68k")
>> >>   * commit 9e2d3cd34a159948dc753a14573e16bffc04dba8 ("[PATCH]
>> >> mod_devicetable.h fixes")
>> >
>> > So would the patch below fix this?  It should force alignment of the
>> > driver_data field, which is all you want here, right?
>> >
>> >> Still, there's a bug in file2alias (which is compiled by the host
>> >> compiler), in that
>> >> it may use different padding than the target platform when cross-compiling.
>> >
>> > That's not good, but outside of this specific issue, right?  Have we
>> > just been fortunate it hasn't really hit us yet?
>> >
>> > thanks,
>> >
>> > greg k-h
>> >
>> > diff --git a/include/linux/mod_devicetable.h b/include/linux/mod_devicetable.h
>> > index 7771d45..6955045 100644
>> > --- a/include/linux/mod_devicetable.h
>> > +++ b/include/linux/mod_devicetable.h
>> > @@ -122,7 +122,8 @@ struct usb_device_id {
>> >     __u8            bInterfaceNumber;
>> >
>> >     /* not matched against */
>> > -   kernel_ulong_t  driver_info;
>> > +   kernel_ulong_t  driver_info
>> > +           __attribute__((aligned(sizeof(kernel_ulong_t))));
>> >  };
>>
>>
>> This feels a lot like papering over the real problem.  It will solve
>> this instance, but the list of such previous "paper work" that Geert
>> provided should be enough evidence that this will happen again the next
>> time someone modifies a device id struct for some subsystem.
>
> Hopefully not, if you add another field here, the alignment force will
> keep things lined up properly, from what I can tell.  Is that not true?

... for struct usb_device_id.
But not for all other existing and future device ID types.

We have kernel_ulong_t to tell the host compiler what the target's
unsigned long type (actually only its size, not alignment) is.

scripts/mod/file2alias.c handles this with:

#if KERNEL_ELFCLASS == ELFCLASS32
typedef Elf32_Addr      kernel_ulong_t;
#define BITS_PER_LONG 32
#else
typedef Elf64_Addr      kernel_ulong_t;
#define BITS_PER_LONG 64
#endif

To fix the misalignment issue, can't we add
"__attribute__((aligned(2)))" to the typedef when cross-compiling for m68k?

Still, that only solves the problem for "kernel_ulong_t". Not for all other
4 or 8 bytes types (in practice just "u32") that are used in
include/linux/mod_devicetable.h. So we would also need "kernel_u32".
And "kernel_u16" for consistency.

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/