* [bug report] media: lirc: improve locking
@ 2017-12-13 10:07 Dan Carpenter
2017-12-13 13:57 ` Sean Young
0 siblings, 1 reply; 2+ messages in thread
From: Dan Carpenter @ 2017-12-13 10:07 UTC (permalink / raw)
To: sean; +Cc: linux-media
Hello Sean Young,
The patch 131fd7fc3c01: "media: lirc: improve locking" from Nov 4,
2017, leads to the following static checker warning:
drivers/media/rc/lirc_dev.c:373 ir_lirc_transmit_ir()
error: 'txbuf' dereferencing possible ERR_PTR()
drivers/media/rc/lirc_dev.c
330 txbuf = memdup_user(buf, n);
331 if (IS_ERR(txbuf)) {
332 ret = PTR_ERR(txbuf);
333 goto out;
^^^^^^^^
This used to be a direct return...
334 }
335 }
336
337 for (i = 0; i < count; i++) {
338 if (txbuf[i] > IR_MAX_DURATION / 1000 - duration || !txbuf[i]) {
339 ret = -EINVAL;
340 goto out;
341 }
342
343 duration += txbuf[i];
344 }
345
346 ret = dev->tx_ir(dev, txbuf, count);
347 if (ret < 0)
348 goto out;
349
350 if (fh->send_mode == LIRC_MODE_SCANCODE) {
351 ret = n;
352 } else {
353 for (duration = i = 0; i < ret; i++)
354 duration += txbuf[i];
355
356 ret *= sizeof(unsigned int);
357
358 /*
359 * The lircd gap calculation expects the write function to
360 * wait for the actual IR signal to be transmitted before
361 * returning.
362 */
363 towait = ktime_us_delta(ktime_add_us(start, duration),
364 ktime_get());
365 if (towait > 0) {
366 set_current_state(TASK_INTERRUPTIBLE);
367 schedule_timeout(usecs_to_jiffies(towait));
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
This looks like a long wait? Are you sure you want to hold the lock
this whole time?
368 }
369 }
370
371 out:
372 mutex_unlock(&dev->lock);
373 kfree(txbuf);
^^^^^
Can't pass an error pointer to kfree().
374 kfree(raw);
regards,
dan carpenter
^ permalink raw reply [flat|nested] 2+ messages in thread
* Re: [bug report] media: lirc: improve locking
2017-12-13 10:07 [bug report] media: lirc: improve locking Dan Carpenter
@ 2017-12-13 13:57 ` Sean Young
0 siblings, 0 replies; 2+ messages in thread
From: Sean Young @ 2017-12-13 13:57 UTC (permalink / raw)
To: Dan Carpenter; +Cc: linux-media
On Wed, Dec 13, 2017 at 01:07:31PM +0300, Dan Carpenter wrote:
> Hello Sean Young,
>
> The patch 131fd7fc3c01: "media: lirc: improve locking" from Nov 4,
> 2017, leads to the following static checker warning:
>
> drivers/media/rc/lirc_dev.c:373 ir_lirc_transmit_ir()
> error: 'txbuf' dereferencing possible ERR_PTR()
>
> drivers/media/rc/lirc_dev.c
> 330 txbuf = memdup_user(buf, n);
> 331 if (IS_ERR(txbuf)) {
> 332 ret = PTR_ERR(txbuf);
> 333 goto out;
> ^^^^^^^^
> This used to be a direct return...
>
> 334 }
> 335 }
> 336
> 337 for (i = 0; i < count; i++) {
> 338 if (txbuf[i] > IR_MAX_DURATION / 1000 - duration || !txbuf[i]) {
> 339 ret = -EINVAL;
> 340 goto out;
> 341 }
> 342
> 343 duration += txbuf[i];
> 344 }
> 345
> 346 ret = dev->tx_ir(dev, txbuf, count);
> 347 if (ret < 0)
> 348 goto out;
> 349
> 350 if (fh->send_mode == LIRC_MODE_SCANCODE) {
> 351 ret = n;
> 352 } else {
> 353 for (duration = i = 0; i < ret; i++)
> 354 duration += txbuf[i];
> 355
> 356 ret *= sizeof(unsigned int);
> 357
> 358 /*
> 359 * The lircd gap calculation expects the write function to
> 360 * wait for the actual IR signal to be transmitted before
> 361 * returning.
> 362 */
> 363 towait = ktime_us_delta(ktime_add_us(start, duration),
> 364 ktime_get());
> 365 if (towait > 0) {
> 366 set_current_state(TASK_INTERRUPTIBLE);
> 367 schedule_timeout(usecs_to_jiffies(towait));
> ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
> This looks like a long wait? Are you sure you want to hold the lock
> this whole time?
Good point, there is no need for that.
> 368 }
> 369 }
> 370
> 371 out:
> 372 mutex_unlock(&dev->lock);
> 373 kfree(txbuf);
> ^^^^^
> Can't pass an error pointer to kfree().
Yep.
Thank you for the report. I'll write patch.
Thanks
Sean
^ permalink raw reply [flat|nested] 2+ messages in thread
end of thread, other threads:[~2017-12-13 13:57 UTC | newest]
Thread overview: 2+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2017-12-13 10:07 [bug report] media: lirc: improve locking Dan Carpenter
2017-12-13 13:57 ` Sean Young
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox