linux-kernel - Re: Linux 3.3-rc5

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: <CA+55aFxMPOAdvu-iFL8vDzC=d1osVD91RfksLiE4sAP5Lf5oNQ@mail.gmail.com>
Date:	Tue, 28 Feb 2012 10:30:30 -0800
From:	Linus Torvalds <torvalds@...ux-foundation.org>
To:	Mark Lord <kernel@...savvy.com>
Cc:	Linux Kernel Mailing List <linux-kernel@...r.kernel.org>
Subject: Re: Linux 3.3-rc5

On Mon, Feb 27, 2012 at 7:23 AM, Mark Lord <kernel@...savvy.com> wrote:
>
> The i8k driver is still b0rked for COMPAT use in linux-3.2.xx,
> and I don't think my patch got picked up by anyone for 3.3 yet:

I really don't like your patch.

It basically compares the ioctl argument by dropping almost all bits.
That looks completely wrong. If I read the patch correctly, it will
now match ioctl's that have *nothing* to do with the i8k ioctls, that
just happen to have a few bits in common.

I also think that your explanation is wrong. The problem is not the
use of _IOR() at all, the problem is that the data structure given
*to* the _IOR() macro is complete grabage. For example:

  #define I8K_GET_SPEED           _IOWR('i', 0x85, size_t)

that "size_t" there is just crap. It's wrong. The ioctl doesn't write
a size_t (which is different sizes on 32-bit and 64-bit), it actually
writes an "int" (which happens to be the same size in both compat and
non-compat).

So the 64-bit code - totally incorrectly - has been compiled with an
ioctl number that implies a 64-bit argument.

EVERY SINGLE IOCTL that is defined for the i8k driver is crap. There
are two that were "int", but they are marked as "broken" because the
data structure *they* write isn't actually "int". But those two are
the ones that just happen to work in both 32-bit and 64-bit mode,
because at least the size of the (incorrect) data structure is the
same.

The ones that have "size_t" aren't marked broken, but those are the
*really* broken ones due to the wrong data structure choice that has a
size that now is different in 32-bit and 64-bit mode.

Quite frankly, I think the right solution is to fix the kernel
interface to the right type (int) that is the same. But because we
don't want to change the user interface, let's make the kernel
*accept* the 8-byte entity and just change it into a 4-byte size, and
leave the user-space visible ioctl numbers the same broken crap - it's
not like the other ioctl numbers had the right size *either*...

IOW, have something like the attached in the ioctl handler (and then
we need to also add the compat handler, as in your patch).

Does this (with your compat_ioctl wrapper addition) also work for you?

                        Linus

View attachment "patch.diff" of type "text/x-patch" (1907 bytes)