Re: [CHECKER] Probable security holes in 2.6.5

[Chris Wright <chrisw@osdl.org>]

> [BUG] minor
> /home/kash/linux/linux-2.6.5/drivers/md/dm-ioctl.c:1180:copy_params:
> ERROR:TAINT: 1174:1180:Passing unbounded user value "(tmp).data_size" as
> arg 0 to function "vmalloc", which uses it unsafely in model
> [SOURCE_MODEL=(lib,copy_from_user,user,taintscalar)]
> [SINK_MODEL=(lib,vmalloc,user,trustingsink)] [BOUNDS= Lower bound on
> line 1177] [MINOR]  [PATH=] 
> 
> static int copy_params(struct dm_ioctl *user, struct dm_ioctl **param)
> {
> 	struct dm_ioctl tmp, *dmi;
> 
> Start --->
> 	if (copy_from_user(&tmp, user, sizeof(tmp)))
> 		return -EFAULT;
> 
> 	if (tmp.data_size < sizeof(tmp))
> 		return -EINVAL;
> 
> Error --->
> 	dmi = (struct dm_ioctl *) vmalloc(tmp.data_size);
> 	if (!dmi)
> 		return -ENOMEM;
> 

Looks like dm_ioctl() has a free form untyped buffer at the end of the
dm_ioctl struct, which makes it rough to figure the appropriate max for
data_size, esp, those that can be a list.  It's protected by capable(),
not clear if there's a good fix, and I don't see an overflow just a way
to vmalloc() a large bit of memory.  Perhaps there's a case where one
could rename to a name larger than DM_NAME_LEN, then no longer be able to
lookup based on ->name (only ->uuid).

thanks,
-chris
-- 
Linux Security Modules     http://lsm.immunix.org     http://lsm.bkbits.net