linux-kernel - Re: [PATCH] ALSA: compress: Fix the mismatch size of struc between share lib(32bit) and kernel(64bit)

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 for Android: free password hash cracker in your pocket

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-Id: <201406131619.32976.arnd@arndb.de>
Date:	Fri, 13 Jun 2014 16:19:32 +0200
From:	Arnd Bergmann <arnd@...db.de>
To:	Vinod Koul <vinod.koul@...el.com>
Cc:	Takashi Iwai <tiwai@...e.de>,
	"Wang, Xiaoming" <xiaoming.wang@...el.com>, jeeja.kp@...el.com,
	dhowells@...hat.com, tglx@...utronix.de, mtk.manpages@...il.com,
	paulmck@...ux.vnet.ibm.com, davej@...hat.com,
	linux-kernel@...r.kernel.org, dongxing.zhang@...el.com
Subject: Re: [PATCH] ALSA: compress: Fix the mismatch size of struc between share lib(32bit) and kernel(64bit)

On Monday 09 June 2014, Vinod Koul wrote:
> On Mon, Jun 09, 2014 at 09:24:53AM +0200, Takashi Iwai wrote:
> > At Mon, 09 Jun 2014 16:46:32 -0400,
> > Wang, Xiaoming wrote:
> > > 
> > > 
> > > The size of struct snd_compr_avail is 0x1c in 32bit kernel,
> > > while it is 0x20 in 64bit kernel 0x4 bytes added because of
> > > alignment. It is OK when 32bit kernel met 32bit user space.
> > > There exist stack corruption if 64bit kernel met 32bit user
> > > space, because the size of struct snd_compr_avail is 0x1c
> > > in 32bit user space which is smaller than it will get from
> > > kernel. The extra 4 bytes can corrupt the stack, and
> > > introduce unpredictable error.
> > > 
> > > Signed-off-by: Zhang Dongxing <dongxing.zhang@...el.com>
> > > Signed-off-by: xiaoming wang <xiaoming.wang@...el.com>
> > 
> > This would break the existing 32bit systems, so I don't think we can
> > take this approach.
> > 
> > Either break the 64bit systems (which aren't deployed yet much, so
> > far) by adding packed attribute, or implement 32/64 bit conversion in
> > compat_ioctl fop.
> I think former should be safe for now. Anyway we have only 1 driver using this
> in mainline so fallout shouldn't be widespread!

I guess since the driver was only merged for 3.16, we don't
really have to worry about the ABI breakage anyway, but you can
still use that approach if you have reason to stay compatible with
existing used space built against out-of-tree drivers.

Anyway, if you use the __packed attribute, best apply it only to
the individual __u64 member(s), not the entire struct, otherwise
you might change user space programs in a subtle way when the alignment
changes from 4 to 1 byte. 

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