linux-kernel - Re: [PATCH 2/2] Input: Add EVIOC mechanism for MT slots

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] [day] [month] [year] [list]

Message-ID: <20120123095248.GA6667@polaris.bitmath.org>
Date:	Mon, 23 Jan 2012 10:52:48 +0100
From:	"Henrik Rydberg" <rydberg@...omail.se>
To:	Dmitry Torokhov <dmitry.torokhov@...il.com>
Cc:	Chase Douglas <chase.douglas@...onical.com>,
	linux-input@...r.kernel.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH 2/2] Input: Add EVIOC mechanism for MT slots

Hi Dmitry,

> > > +static int evdev_handle_mt_request(struct input_dev *dev,
> > > +				   unsigned int size,
> > > +				   int __user *ip)
> > > +{
> > > +	const struct input_mt_slot *mt = dev->mt;
> > > +	unsigned int code;
> > > +	int i;
> > > +
> > > +	if (get_user(code, &ip[0]))
> > > +		return -EFAULT;
> > > +	if (!input_is_mt_value(code))
> > > +		return -EINVAL;
> > > +
> > > +	if (size < sizeof(struct input_mt_request) + dev->mtsize * sizeof(int))
> > > +		return -EINVAL;
> > 
> > Why not fill in up to the size of the array provided?
> 
> Yes, I think we do the same with other ioctls as well.

Agreed, will do it that way.

> > > +/**
> > > + * struct input_mt_request - used by EVIOCGMTSLOTS ioctl
> > > + * @code: ABS_MT code to read
> > > + * @num_values: number of values written to the value array
> > > + * @values: value array, one value per slot
> > > + *
> > > + * The structure is used to retrieve MT slot data. Before the call,
> > > + * code is set to the wanted ABS_MT event type. On return, the value
> > > + * array is filled with the slot values for the specified ABS_MT code,
> > > + * and num_values is set to the actual number of slots.
> > > + *
> > > + * The call argument (len) is the size of the return buffer, satisfying
> > > + *
> > > + * len >= sizeof(input_mt_request) + sizeof(__s32) * number_of_slots
> > > + *
> > > + * If the request code is not an ABS_MT value, or if len is too small,
> > > + * -EINVAL is returned.
> > > + */
> > > +struct input_mt_request {
> > > +	__u32 code;
> > > +	__u32 num_values;
> 
> Hmm, might avoid this one... Userspace can do EVIOCGABS to get number of
> slots and then it will know how much data EVIOCGMTSLOTS will return...

I think it might be good to keep the length, to avoid possible future
synchronization problems. Perhaps we could get rid of the structure
though, for instance by specifying the write as a 32-bit code and the
read buffer as length, value, value...

Thanks,
Henrik
--
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/