[PATCH 0/2] Input: uinput - fix ioctl numbers in uapi/uinput.h

[PATCH 0/2] Input: uinput - fix ioctl numbers in uapi/uinput.h

[Gabriel Laskar]

Ioctls numbers for UI_GET_SYSNAME and UI_GET_VERSION are incorrectly numbered,
since nr number is 8bit encoded, 300 and 301 will effectively get 44 and 45.
these two patches fixes this

Gabriel Laskar (2):
  Input: uinput - fix ioctl nr overflow for UI_GET_SYSNAME
  Input: uinput - fix ioctl nr overflow for UI_GET_VERSION

 include/uapi/linux/uinput.h | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

-- 
2.2.1

[PATCH 1/2] Input: uinput - fix ioctl nr overflow for UI_GET_SYSNAME

[Gabriel Laskar]

Request number for ioctls are encoded on 8bit. Values for are superior
to 255. The effective value is 0x2c. The effective ioctl number is still
the same one, it will not change the api in anyway.

Signed-off-by: Gabriel Laskar <gabriel-tU7rkvAWjlwhT4uAktR2oQ@public.gmane.org>
---
 include/uapi/linux/uinput.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/include/uapi/linux/uinput.h b/include/uapi/linux/uinput.h
index baeab83..358f7d9 100644
--- a/include/uapi/linux/uinput.h
+++ b/include/uapi/linux/uinput.h
@@ -82,7 +82,7 @@ struct uinput_ff_erase {
  * The complete sysfs path is then /sys/devices/virtual/input/--NAME--
  * Usually, it is in the form "inputN"
  */
-#define UI_GET_SYSNAME(len)	_IOC(_IOC_READ, UINPUT_IOCTL_BASE, 300, len)
+#define UI_GET_SYSNAME(len)	_IOC(_IOC_READ, UINPUT_IOCTL_BASE, 0x2c, len)
 
 /**
  * UI_GET_VERSION - Return version of uinput protocol
-- 
2.2.1

[PATCH 2/2] Input: uinput - fix ioctl nr overflow for UI_GET_VERSION

[Gabriel Laskar]

Request number for ioctls are encoded on 8bit. Values for are superior
to 255. The effective value is 0x2d. The effective ioctl number is still
the same one, it will not change the api in anyway.

Signed-off-by: Gabriel Laskar <gabriel-tU7rkvAWjlwhT4uAktR2oQ@public.gmane.org>
---
 include/uapi/linux/uinput.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/include/uapi/linux/uinput.h b/include/uapi/linux/uinput.h
index 358f7d9..e1daf2e 100644
--- a/include/uapi/linux/uinput.h
+++ b/include/uapi/linux/uinput.h
@@ -91,7 +91,7 @@ struct uinput_ff_erase {
  * the integer pointed to by the ioctl argument. The protocol version
  * is hard-coded in the kernel and is independent of the uinput device.
  */
-#define UI_GET_VERSION		_IOR(UINPUT_IOCTL_BASE, 301, unsigned int)
+#define UI_GET_VERSION		_IOR(UINPUT_IOCTL_BASE, 0x2d, unsigned int)
 
 /*
  * To write a force-feedback-capable driver, the upload_effect
-- 
2.2.1

Re: [PATCH 2/2] Input: uinput - fix ioctl nr overflow for UI_GET_VERSION

[Peter Hutterer]

On Sat, Jan 10, 2015 at 01:43:36PM +0100, Gabriel Laskar wrote:
> Request number for ioctls are encoded on 8bit. Values for are superior
> to 255. The effective value is 0x2d. The effective ioctl number is still
> the same one, it will not change the api in anyway.
> 
> Signed-off-by: Gabriel Laskar <gabriel-tU7rkvAWjlwhT4uAktR2oQ@public.gmane.org>
> ---
>  include/uapi/linux/uinput.h | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/include/uapi/linux/uinput.h b/include/uapi/linux/uinput.h
> index 358f7d9..e1daf2e 100644
> --- a/include/uapi/linux/uinput.h
> +++ b/include/uapi/linux/uinput.h
> @@ -91,7 +91,7 @@ struct uinput_ff_erase {
>   * the integer pointed to by the ioctl argument. The protocol version
>   * is hard-coded in the kernel and is independent of the uinput device.
>   */
> -#define UI_GET_VERSION		_IOR(UINPUT_IOCTL_BASE, 301, unsigned int)
> +#define UI_GET_VERSION		_IOR(UINPUT_IOCTL_BASE, 0x2d, unsigned int)

the other uinput ioctls have the number specified as decimal number, I think
the same should be done here.

Also, certainly dodged a bullet there: 301 overflows into the type field but
sets a bit that is already set by UINPUT_IOCTL_BASE so it has no effect
(which I guess is why we didn't spot this before). ACK to the patch (when
changed to decimal) but it seems applying the masks in the _IOC macro may be
prudent to avoid this in the future.

Cheers,
   Peter

>  
>  /*
>   * To write a force-feedback-capable driver, the upload_effect
> -- 
> 2.2.1
> 

Re: [PATCH 0/2] Input: uinput - fix ioctl numbers in uapi/uinput.h

[Dmitry Torokhov]

Hi Gabriel,

On Sat, Jan 10, 2015 at 01:43:34PM +0100, Gabriel Laskar wrote:
> Ioctls numbers for UI_GET_SYSNAME and UI_GET_VERSION are incorrectly numbered,
> since nr number is 8bit encoded, 300 and 301 will effectively get 44 and 45.
> these two patches fixes this
> 

Nice catch, thank you! I folded the patches together (as they are fixing
essentially the same thing) , changed hex to dec (because the rest of
ioctls in uinput use decimal) and applied.

I wonder if we need to put a BUILD_BUG_ON in one if _IO* defines to
catch such errors early on.

Thanks.

-- 
Dmitry

Re: [PATCH 0/2] Input: uinput - fix ioctl numbers in uapi/uinput.h

[Benjamin Tissoires]

Hi,

On Jan 11 2015 or thereabouts, Dmitry Torokhov wrote:
> Hi Gabriel,
> 
> On Sat, Jan 10, 2015 at 01:43:34PM +0100, Gabriel Laskar wrote:
> > Ioctls numbers for UI_GET_SYSNAME and UI_GET_VERSION are incorrectly numbered,
> > since nr number is 8bit encoded, 300 and 301 will effectively get 44 and 45.
> > these two patches fixes this
> > 
> 
> Nice catch, thank you! I folded the patches together (as they are fixing
> essentially the same thing) , changed hex to dec (because the rest of
> ioctls in uinput use decimal) and applied.
> 
> I wonder if we need to put a BUILD_BUG_ON in one if _IO* defines to
> catch such errors early on.
> 

As others said, nice catch, thanks a lot. I feel very embarrassed about
this :(

Cheers,
Benjamin