[PATCH] input: avoid negative input device numbers

[PATCH] input: avoid negative input device numbers

[Richard Leitner]

From: Richard Leitner <richard.leitner@skidata.com>

Fix the format string for input device name generation to avoid negative
device numbers when the id exceeds the maximum signed integer value.

Signed-off-by: Richard Leitner <richard.leitner@skidata.com>
---
 drivers/input/input.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/input/input.c b/drivers/input/input.c
index 236bc56..6be6982 100644
--- a/drivers/input/input.c
+++ b/drivers/input/input.c
@@ -1791,7 +1791,7 @@ struct input_dev *input_allocate_device(void)
 		INIT_LIST_HEAD(&dev->h_list);
 		INIT_LIST_HEAD(&dev->node);
 
-		dev_set_name(&dev->dev, "input%ld",
+		dev_set_name(&dev->dev, "input%lu",
 			     (unsigned long) atomic_inc_return(&input_no) - 1);
 
 		__module_get(THIS_MODULE);
-- 
2.1.1

[RFC] avoid (theoretical) conflicts of input device file names

[Richard Leitner]

Hi,
currently I discovered the possibility that device file numbers of the input
subsystem could go negative when the signed int "border" is passed. To fix
this behaviour I sent a patch a few minutes ago.

But as the subject says there is currently the (theoretical) possibility that
the same input device file name is given out twice. This can happen if the
"input_no" variable had an overflow (due to the fact this is at least at 2^32
I call the issue theoretical). If such a case occurs a -EEXISTS is returned at
the creation of the file.

IMHO it would be a good idea to check if the chosen input device file name
is valid at the point it is created (which is currently input_allocate_device).
So you can just increment and check it again until there's a valid number/name
found for it.

I'm pretty new to the input subsystem, so what do you think about it?
Any comments/ideas? Would there be a better place to do such checking?

regards,
richard

Re: [RFC] avoid (theoretical) conflicts of input device file names

[Dmitry Torokhov]

Hi Richard,

On Wed, Oct 08, 2014 at 10:49:29PM +0200, Richard Leitner wrote:
> Hi,
> currently I discovered the possibility that device file numbers of the input
> subsystem could go negative when the signed int "border" is passed. To fix
> this behaviour I sent a patch a few minutes ago.
> 
> But as the subject says there is currently the (theoretical) possibility that
> the same input device file name is given out twice. This can happen if the
> "input_no" variable had an overflow (due to the fact this is at least at 2^32
> I call the issue theoretical). If such a case occurs a -EEXISTS is returned at
> the creation of the file.
> 
> IMHO it would be a good idea to check if the chosen input device file name
> is valid at the point it is created (which is currently input_allocate_device).
> So you can just increment and check it again until there's a valid number/name
> found for it.
> 
> I'm pretty new to the input subsystem, so what do you think about it?
> Any comments/ideas? Would there be a better place to do such checking?

I do not think it is worth checking. Yes, theoretically you can wrap
around, but practically instantiating at least 2^32 devices will take
too long. If ever it becomes a concern my very distant future relatives
will move the counter to 64 or 128 bit and call it a day.

Thanks.

-- 
Dmitry

Re: [PATCH] input: avoid negative input device numbers

[Dmitry Torokhov]

On Wed, Oct 08, 2014 at 10:42:45PM +0200, Richard Leitner wrote:
> From: Richard Leitner <richard.leitner@skidata.com>
> 
> Fix the format string for input device name generation to avoid negative
> device numbers when the id exceeds the maximum signed integer value.

Well, it is going to take us a while to get there :)

Applied, thank you.


> 
> Signed-off-by: Richard Leitner <richard.leitner@skidata.com>
> ---
>  drivers/input/input.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/input/input.c b/drivers/input/input.c
> index 236bc56..6be6982 100644
> --- a/drivers/input/input.c
> +++ b/drivers/input/input.c
> @@ -1791,7 +1791,7 @@ struct input_dev *input_allocate_device(void)
>  		INIT_LIST_HEAD(&dev->h_list);
>  		INIT_LIST_HEAD(&dev->node);
>  
> -		dev_set_name(&dev->dev, "input%ld",
> +		dev_set_name(&dev->dev, "input%lu",
>  			     (unsigned long) atomic_inc_return(&input_no) - 1);
>  
>  		__module_get(THIS_MODULE);
> -- 
> 2.1.1
> 

-- 
Dmitry

Re: [PATCH] input: avoid negative input device numbers

[Dmitry Torokhov]

On Wed, Oct 08, 2014 at 02:25:38PM -0700, Dmitry Torokhov wrote:
> On Wed, Oct 08, 2014 at 10:42:45PM +0200, Richard Leitner wrote:
> > From: Richard Leitner <richard.leitner@skidata.com>
> > 
> > Fix the format string for input device name generation to avoid negative
> > device numbers when the id exceeds the maximum signed integer value.
> 
> Well, it is going to take us a while to get there :)
> 
> Applied, thank you.

By the way, we have similar issue in serio and gameport code, mind
sending fixes for them as well?

Thanks.

-- 
Dmitry

Re: [PATCH] input: avoid negative input device numbers

[Richard Leitner]

On Wed, 8 Oct 2014 14:30:51 -0700
Dmitry Torokhov <dmitry.torokhov@gmail.com> wrote:

> On Wed, Oct 08, 2014 at 02:25:38PM -0700, Dmitry Torokhov wrote:
> > On Wed, Oct 08, 2014 at 10:42:45PM +0200, Richard Leitner wrote:
> > > From: Richard Leitner <richard.leitner@skidata.com>
> > > 
> > > Fix the format string for input device name generation to avoid
> > > negative device numbers when the id exceeds the maximum signed
> > > integer value.
> > 
> > Well, it is going to take us a while to get there :)
> > 
> > Applied, thank you.
> 
> By the way, we have similar issue in serio and gameport code, mind
> sending fixes for them as well?
> 
I'll send a patch for the serio in a few minutes.

The gameport code looks fine to me:

536		dev_set_name(&gameport->dev, "gameport%lu",
537				(unsigned long)atomic_inc_return(&gameport_no) - 1);

regards,
richard