linux-kernel - Re: [PATCH] mISDN cleanup user interface

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: <1217409211.27925.177.camel@pmac.infradead.org>
Date:	Wed, 30 Jul 2008 10:13:31 +0100
From:	David Woodhouse <dwmw2@...radead.org>
To:	Marcel Holtmann <marcel@...tmann.org>
Cc:	Karsten Keil <kkeil@...e.de>,
	Linus Torvalds <torvalds@...ux-foundation.org>,
	linux-kernel@...r.kernel.org, isdn4linux@...tserv.isdn4linux.de
Subject: Re: [PATCH] mISDN cleanup user interface

On Tue, 2008-07-29 at 23:02 +0200, Marcel Holtmann wrote:
> Hi Karsten,
> 
> > > Please pull from git://git.kernel.org/pub/scm/linux/kernel/git/kkeil/ISDN-2.6.git master
> > > 
> > > The channelmap should have the same size on 32 and 64 bit
> > > systems. Thanks to David Woodhouse for spotting this.
> > > 
> > > Signed-off-by: Karsten Keil <kkeil@...e.de>
> > 
> > Er, isn't that still broken? Consider 32-bit userspace on 64-bit kernel,
> > big-endian.
> 
> I agree with David here. Please lets not do these horribly things again.
> Almost everybody has done their fair share of brokenness with the compat
> layers. Especially 32-bit userspace on 64-bit kernels. Using __u32 and
> alike is a way better choice and less headache.

Well, he's already switched to a 32-bit type, which is fine in that
respect -- at least the struct is the same _size_ on 32-bit and 64-bit
targets now. (OK, so inventing new types like 'u_int' instead of just
using the ones that the C language provides is a bit silly, but that's
the Linux norm so I wasn't complaining about that.)

But the channelmap field is still broken because it's using set_bit() on
int types, and they're defined to work on unsigned long. So in the
fairly common case where a BRI driver sets bits 1 and 2 (sic) in the
channel map, that will look like this on a 64-bit big-endian kernel:

 00 00 00 00 00 00 00 06   00 00 00 00 00 00 00 00
    (unsigned long #1)       (unsigned long #2)

When 32-bit userspace receives that, it'll see this:

 00 00 00 00   00 00 00 06   00 00 00 00   00 00 00 00
  (word #1)     (word #2)     (word #3)     (word #4)

.. in which it's bits 33 and 34 that are set. So the new version in the
patch to which I replied _still_ needs a compat routine.

You might get away with it if you use set_le_bit() though.

-- 
David Woodhouse                            Open Source Technology Centre
David.Woodhouse@...el.com                              Intel Corporation

--
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/