* re: [PATCH] drivers/input/joystick: convert to dynamic input_dev allocation
@ 2015-07-28 17:00 Dan Carpenter
2015-07-30 18:04 ` Dmitry Torokhov
0 siblings, 1 reply; 4+ messages in thread
From: Dan Carpenter @ 2015-07-28 17:00 UTC (permalink / raw)
To: Dmitry Torokhov; +Cc: linux-input
Hello Dmitry Torokhov,
The patch 17dd3f0f7aa7: "[PATCH] drivers/input/joystick: convert to
dynamic input_dev allocation" from Sep 15, 2005, leads to the
following static checker warning:
drivers/input/joystick/turbografx.c:235 tgfx_probe()
error: buffer overflow 'tgfx_buttons' 5 <= 5
drivers/input/joystick/turbografx.c
195 for (i = 0; i < n_devs; i++) {
196 if (n_buttons[i] < 1)
197 continue;
198
199 if (n_buttons[i] > 6) {
^^^^^^^^^^^^^^^^
Possibly off by one. >= 6.
200 printk(KERN_ERR "turbografx.c: Invalid number of buttons %d\n", n_buttons[i]);
201 err = -EINVAL;
202 goto err_unreg_devs;
203 }
204
205 tgfx->dev[i] = input_dev = input_allocate_device();
206 if (!input_dev) {
207 printk(KERN_ERR "turbografx.c: Not enough memory for input device\n");
208 err = -ENOMEM;
209 goto err_unreg_devs;
210 }
211
212 tgfx->sticks |= (1 << i);
213 snprintf(tgfx->name[i], sizeof(tgfx->name[i]),
214 "TurboGraFX %d-button Multisystem joystick", n_buttons[i]);
215 snprintf(tgfx->phys[i], sizeof(tgfx->phys[i]),
216 "%s/input%d", tgfx->pd->port->name, i);
217
218 input_dev->name = tgfx->name[i];
219 input_dev->phys = tgfx->phys[i];
220 input_dev->id.bustype = BUS_PARPORT;
221 input_dev->id.vendor = 0x0003;
222 input_dev->id.product = n_buttons[i];
223 input_dev->id.version = 0x0100;
224
225 input_set_drvdata(input_dev, tgfx);
226
227 input_dev->open = tgfx_open;
228 input_dev->close = tgfx_close;
229
230 input_dev->evbit[0] = BIT_MASK(EV_KEY) | BIT_MASK(EV_ABS);
231 input_set_abs_params(input_dev, ABS_X, -1, 1, 0, 0);
232 input_set_abs_params(input_dev, ABS_Y, -1, 1, 0, 0);
233
234 for (j = 0; j < n_buttons[i]; j++)
235 set_bit(tgfx_buttons[j], input_dev->keybit);
^^^^^^^^^^^^^^^
Leading to an off by one write here. This only has 5 elements.
236
regards,
dan carpenter
^ permalink raw reply [flat|nested] 4+ messages in thread* Re: [PATCH] drivers/input/joystick: convert to dynamic input_dev allocation
2015-07-28 17:00 [PATCH] drivers/input/joystick: convert to dynamic input_dev allocation Dan Carpenter
@ 2015-07-30 18:04 ` Dmitry Torokhov
2015-07-30 19:15 ` Dan Carpenter
0 siblings, 1 reply; 4+ messages in thread
From: Dmitry Torokhov @ 2015-07-30 18:04 UTC (permalink / raw)
To: Dan Carpenter; +Cc: linux-input
On Tue, Jul 28, 2015 at 08:00:24PM +0300, Dan Carpenter wrote:
> Hello Dmitry Torokhov,
>
> The patch 17dd3f0f7aa7: "[PATCH] drivers/input/joystick: convert to
> dynamic input_dev allocation" from Sep 15, 2005, leads to the
> following static checker warning:
>
> drivers/input/joystick/turbografx.c:235 tgfx_probe()
> error: buffer overflow 'tgfx_buttons' 5 <= 5
>
> drivers/input/joystick/turbografx.c
> 195 for (i = 0; i < n_devs; i++) {
> 196 if (n_buttons[i] < 1)
> 197 continue;
> 198
> 199 if (n_buttons[i] > 6) {
> ^^^^^^^^^^^^^^^^
> Possibly off by one. >= 6.
How about this:
Input: turbografx - fix potential out of bound access
From: Dmitry Torokhov <dmitry.torokhov@gmail.com>
Patch 17dd3f0f7aa7: "[PATCH] drivers/input/joystick: convert to dynamic
input_dev allocation" from Sep 15, 2005, leads to the following static
checker warning:
drivers/input/joystick/turbografx.c:235 tgfx_probe()
error: buffer overflow 'tgfx_buttons' 5 <= 5
drivers/input/joystick/turbografx.c
195 for (i = 0; i < n_devs; i++) {
196 if (n_buttons[i] < 1)
197 continue;
198
199 if (n_buttons[i] > 6) {
^^^^^^^^^^^^^^^^
Possibly off by one. >= 6.
Let's change the upper value to ARRAY_SIZE(tgfx_buttons) to ensure we do
not reach past the end of the array.
Reported-by: Dan Carpenter <dan.carpenter@oracle.com>
Signed-off-by: Dmitry Torokhov <dmitry.torokhov@gmail.com>
---
drivers/input/joystick/turbografx.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/input/joystick/turbografx.c b/drivers/input/joystick/turbografx.c
index 27b6a3c..891797a 100644
--- a/drivers/input/joystick/turbografx.c
+++ b/drivers/input/joystick/turbografx.c
@@ -196,7 +196,7 @@ static struct tgfx __init *tgfx_probe(int parport, int *n_buttons, int n_devs)
if (n_buttons[i] < 1)
continue;
- if (n_buttons[i] > 6) {
+ if (n_buttons[i] > ARRAY_SIZE(tgfx_buttons)) {
printk(KERN_ERR "turbografx.c: Invalid number of buttons %d\n", n_buttons[i]);
err = -EINVAL;
goto err_unreg_devs;
Thanks.
--
Dmitry
^ permalink raw reply related [flat|nested] 4+ messages in thread
end of thread, other threads:[~2015-07-30 19:38 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2015-07-28 17:00 [PATCH] drivers/input/joystick: convert to dynamic input_dev allocation Dan Carpenter
2015-07-30 18:04 ` Dmitry Torokhov
2015-07-30 19:15 ` Dan Carpenter
2015-07-30 19:38 ` Dmitry Torokhov
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).