* [PATCH 1/2] media: rc: uevent sysfs file races with rc_register_device()
@ 2020-08-08 12:31 Sean Young
2020-08-08 12:31 ` [PATCH 2/2] media: rc: do not access device via sysfs after rc_unregister_device() Sean Young
2020-08-08 14:42 ` kernel test robot
0 siblings, 2 replies; 5+ messages in thread
From: Sean Young @ 2020-08-08 12:31 UTC (permalink / raw)
To: linux-media; +Cc: syzbot+ceef16277388d6f24898, Hillf Danton
Only report uevent file contents if device still exists, else we might
read freed memory.
Reported-by: syzbot+ceef16277388d6f24898@syzkaller.appspotmail.com
Cc: Hillf Danton <hdanton@sina.com>
Signed-off-by: Sean Young <sean@mess.org>
---
drivers/media/rc/rc-main.c | 12 ++++++++++--
1 file changed, 10 insertions(+), 2 deletions(-)
diff --git a/drivers/media/rc/rc-main.c b/drivers/media/rc/rc-main.c
index 7b53066d9d07..503ae4f3dec3 100644
--- a/drivers/media/rc/rc-main.c
+++ b/drivers/media/rc/rc-main.c
@@ -1612,6 +1612,12 @@ static int rc_dev_uevent(struct device *device, struct kobj_uevent_env *env)
{
struct rc_dev *dev = to_rc_dev(device);
+ mutex_lock(&dev->lock);
+ if (!dev->registered) {
+ mutex_unlock(&dev->lock);
+ return -ENODEV;
+ }
+
if (dev->rc_map.name)
ADD_HOTPLUG_VAR("NAME=%s", dev->rc_map.name);
if (dev->driver_name)
@@ -1619,6 +1625,8 @@ static int rc_dev_uevent(struct device *device, struct kobj_uevent_env *env)
if (dev->device_name)
ADD_HOTPLUG_VAR("DEV_NAME=%s", dev->device_name);
+ mutex_unlock(&dev->lock);
+
return 0;
}
@@ -2011,14 +2019,14 @@ void rc_unregister_device(struct rc_dev *dev)
del_timer_sync(&dev->timer_keyup);
del_timer_sync(&dev->timer_repeat);
- rc_free_rx_device(dev);
-
mutex_lock(&dev->lock);
if (dev->users && dev->close)
dev->close(dev);
dev->registered = false;
mutex_unlock(&dev->lock);
+ rc_free_rx_device(dev);
+
/*
* lirc device should be freed with dev->registered = false, so
* that userspace polling will get notified.
--
2.26.2
^ permalink raw reply related [flat|nested] 5+ messages in thread* [PATCH 2/2] media: rc: do not access device via sysfs after rc_unregister_device()
2020-08-08 12:31 [PATCH 1/2] media: rc: uevent sysfs file races with rc_register_device() Sean Young
@ 2020-08-08 12:31 ` Sean Young
2020-08-08 14:42 ` kernel test robot
1 sibling, 0 replies; 5+ messages in thread
From: Sean Young @ 2020-08-08 12:31 UTC (permalink / raw)
To: linux-media
Device drivers do not expect to have change_protocol or wakeup
re-programming to be accesed after rc_unregister_device(). This can
cause the device driver to access deallocated resources.
Signed-off-by: Sean Young <sean@mess.org>
---
drivers/media/rc/rc-main.c | 12 ++++++++++++
1 file changed, 12 insertions(+)
diff --git a/drivers/media/rc/rc-main.c b/drivers/media/rc/rc-main.c
index 503ae4f3dec3..c629941f309a 100644
--- a/drivers/media/rc/rc-main.c
+++ b/drivers/media/rc/rc-main.c
@@ -1292,6 +1292,10 @@ static ssize_t store_protocols(struct device *device,
}
mutex_lock(&dev->lock);
+ if (!dev->registered) {
+ mutex_unlock(&dev->lock);
+ return -ENODEV;
+ }
old_protocols = *current_protocols;
new_protocols = old_protocols;
@@ -1430,6 +1434,10 @@ static ssize_t store_filter(struct device *device,
return -EINVAL;
mutex_lock(&dev->lock);
+ if (!dev->registered) {
+ mutex_unlock(&dev->lock);
+ return -ENODEV;
+ }
new_filter = *filter;
if (fattr->mask)
@@ -1544,6 +1552,10 @@ static ssize_t store_wakeup_protocols(struct device *device,
int i;
mutex_lock(&dev->lock);
+ if (!dev->registered) {
+ mutex_unlock(&dev->lock);
+ return -ENODEV;
+ }
allowed = dev->allowed_wakeup_protocols;
--
2.26.2
^ permalink raw reply related [flat|nested] 5+ messages in thread* Re: [PATCH 1/2] media: rc: uevent sysfs file races with rc_register_device()
2020-08-08 12:31 [PATCH 1/2] media: rc: uevent sysfs file races with rc_register_device() Sean Young
@ 2020-08-08 14:42 ` kernel test robot
2020-08-08 14:42 ` kernel test robot
1 sibling, 0 replies; 5+ messages in thread
From: kernel test robot @ 2020-08-08 14:42 UTC (permalink / raw)
To: kbuild-all
[-- Attachment #1: Type: text/plain, Size: 3357 bytes --]
Hi Sean,
I love your patch! Perhaps something to improve:
[auto build test WARNING on linuxtv-media/master]
[also build test WARNING on v5.8 next-20200807]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch]
url: https://github.com/0day-ci/linux/commits/Sean-Young/media-rc-uevent-sysfs-file-races-with-rc_register_device/20200808-203329
base: git://linuxtv.org/media_tree.git master
config: x86_64-randconfig-m001-20200808 (attached as .config)
compiler: gcc-9 (Debian 9.3.0-15) 9.3.0
If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>
smatch warnings:
drivers/media/rc/rc-main.c:1630 rc_dev_uevent() warn: inconsistent returns 'dev->lock'.
vim +1630 drivers/media/rc/rc-main.c
d8b4b5822f51e2 David Härdeman 2010-10-29 1603
bc2a6c5719efd7 Mauro Carvalho Chehab 2010-11-09 1604 #define ADD_HOTPLUG_VAR(fmt, val...) \
bc2a6c5719efd7 Mauro Carvalho Chehab 2010-11-09 1605 do { \
bc2a6c5719efd7 Mauro Carvalho Chehab 2010-11-09 1606 int err = add_uevent_var(env, fmt, val); \
bc2a6c5719efd7 Mauro Carvalho Chehab 2010-11-09 1607 if (err) \
bc2a6c5719efd7 Mauro Carvalho Chehab 2010-11-09 1608 return err; \
bc2a6c5719efd7 Mauro Carvalho Chehab 2010-11-09 1609 } while (0)
bc2a6c5719efd7 Mauro Carvalho Chehab 2010-11-09 1610
bc2a6c5719efd7 Mauro Carvalho Chehab 2010-11-09 1611 static int rc_dev_uevent(struct device *device, struct kobj_uevent_env *env)
bc2a6c5719efd7 Mauro Carvalho Chehab 2010-11-09 1612 {
d8b4b5822f51e2 David Härdeman 2010-10-29 1613 struct rc_dev *dev = to_rc_dev(device);
bc2a6c5719efd7 Mauro Carvalho Chehab 2010-11-09 1614
e4b7677d34e789 Sean Young 2020-08-08 1615 mutex_lock(&dev->lock);
e4b7677d34e789 Sean Young 2020-08-08 1616 if (!dev->registered) {
e4b7677d34e789 Sean Young 2020-08-08 1617 mutex_unlock(&dev->lock);
e4b7677d34e789 Sean Young 2020-08-08 1618 return -ENODEV;
e4b7677d34e789 Sean Young 2020-08-08 1619 }
e4b7677d34e789 Sean Young 2020-08-08 1620
b088ba658b3438 Mauro Carvalho Chehab 2010-11-17 1621 if (dev->rc_map.name)
b088ba658b3438 Mauro Carvalho Chehab 2010-11-17 1622 ADD_HOTPLUG_VAR("NAME=%s", dev->rc_map.name);
d8b4b5822f51e2 David Härdeman 2010-10-29 1623 if (dev->driver_name)
d8b4b5822f51e2 David Härdeman 2010-10-29 1624 ADD_HOTPLUG_VAR("DRV_NAME=%s", dev->driver_name);
b9f407e31c5073 Sean Young 2017-09-01 1625 if (dev->device_name)
b9f407e31c5073 Sean Young 2017-09-01 1626 ADD_HOTPLUG_VAR("DEV_NAME=%s", dev->device_name);
bc2a6c5719efd7 Mauro Carvalho Chehab 2010-11-09 1627
e4b7677d34e789 Sean Young 2020-08-08 1628 mutex_unlock(&dev->lock);
e4b7677d34e789 Sean Young 2020-08-08 1629
bc2a6c5719efd7 Mauro Carvalho Chehab 2010-11-09 @1630 return 0;
bc2a6c5719efd7 Mauro Carvalho Chehab 2010-11-09 1631 }
bc2a6c5719efd7 Mauro Carvalho Chehab 2010-11-09 1632
---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all(a)lists.01.org
[-- Attachment #2: config.gz --]
[-- Type: application/gzip, Size: 50215 bytes --]
^ permalink raw reply [flat|nested] 5+ messages in thread* Re: [PATCH 1/2] media: rc: uevent sysfs file races with rc_register_device()
@ 2020-08-08 14:42 ` kernel test robot
0 siblings, 0 replies; 5+ messages in thread
From: kernel test robot @ 2020-08-08 14:42 UTC (permalink / raw)
To: Sean Young, linux-media
Cc: kbuild-all, syzbot+ceef16277388d6f24898, Hillf Danton
[-- Attachment #1: Type: text/plain, Size: 3290 bytes --]
Hi Sean,
I love your patch! Perhaps something to improve:
[auto build test WARNING on linuxtv-media/master]
[also build test WARNING on v5.8 next-20200807]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch]
url: https://github.com/0day-ci/linux/commits/Sean-Young/media-rc-uevent-sysfs-file-races-with-rc_register_device/20200808-203329
base: git://linuxtv.org/media_tree.git master
config: x86_64-randconfig-m001-20200808 (attached as .config)
compiler: gcc-9 (Debian 9.3.0-15) 9.3.0
If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>
smatch warnings:
drivers/media/rc/rc-main.c:1630 rc_dev_uevent() warn: inconsistent returns 'dev->lock'.
vim +1630 drivers/media/rc/rc-main.c
d8b4b5822f51e2 David Härdeman 2010-10-29 1603
bc2a6c5719efd7 Mauro Carvalho Chehab 2010-11-09 1604 #define ADD_HOTPLUG_VAR(fmt, val...) \
bc2a6c5719efd7 Mauro Carvalho Chehab 2010-11-09 1605 do { \
bc2a6c5719efd7 Mauro Carvalho Chehab 2010-11-09 1606 int err = add_uevent_var(env, fmt, val); \
bc2a6c5719efd7 Mauro Carvalho Chehab 2010-11-09 1607 if (err) \
bc2a6c5719efd7 Mauro Carvalho Chehab 2010-11-09 1608 return err; \
bc2a6c5719efd7 Mauro Carvalho Chehab 2010-11-09 1609 } while (0)
bc2a6c5719efd7 Mauro Carvalho Chehab 2010-11-09 1610
bc2a6c5719efd7 Mauro Carvalho Chehab 2010-11-09 1611 static int rc_dev_uevent(struct device *device, struct kobj_uevent_env *env)
bc2a6c5719efd7 Mauro Carvalho Chehab 2010-11-09 1612 {
d8b4b5822f51e2 David Härdeman 2010-10-29 1613 struct rc_dev *dev = to_rc_dev(device);
bc2a6c5719efd7 Mauro Carvalho Chehab 2010-11-09 1614
e4b7677d34e789 Sean Young 2020-08-08 1615 mutex_lock(&dev->lock);
e4b7677d34e789 Sean Young 2020-08-08 1616 if (!dev->registered) {
e4b7677d34e789 Sean Young 2020-08-08 1617 mutex_unlock(&dev->lock);
e4b7677d34e789 Sean Young 2020-08-08 1618 return -ENODEV;
e4b7677d34e789 Sean Young 2020-08-08 1619 }
e4b7677d34e789 Sean Young 2020-08-08 1620
b088ba658b3438 Mauro Carvalho Chehab 2010-11-17 1621 if (dev->rc_map.name)
b088ba658b3438 Mauro Carvalho Chehab 2010-11-17 1622 ADD_HOTPLUG_VAR("NAME=%s", dev->rc_map.name);
d8b4b5822f51e2 David Härdeman 2010-10-29 1623 if (dev->driver_name)
d8b4b5822f51e2 David Härdeman 2010-10-29 1624 ADD_HOTPLUG_VAR("DRV_NAME=%s", dev->driver_name);
b9f407e31c5073 Sean Young 2017-09-01 1625 if (dev->device_name)
b9f407e31c5073 Sean Young 2017-09-01 1626 ADD_HOTPLUG_VAR("DEV_NAME=%s", dev->device_name);
bc2a6c5719efd7 Mauro Carvalho Chehab 2010-11-09 1627
e4b7677d34e789 Sean Young 2020-08-08 1628 mutex_unlock(&dev->lock);
e4b7677d34e789 Sean Young 2020-08-08 1629
bc2a6c5719efd7 Mauro Carvalho Chehab 2010-11-09 @1630 return 0;
bc2a6c5719efd7 Mauro Carvalho Chehab 2010-11-09 1631 }
bc2a6c5719efd7 Mauro Carvalho Chehab 2010-11-09 1632
---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org
[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 50215 bytes --]
^ permalink raw reply [flat|nested] 5+ messages in thread* Re: [PATCH 1/2] media: rc: uevent sysfs file races with rc_register_device()
2020-08-08 14:42 ` kernel test robot
(?)
@ 2020-08-08 18:14 ` Sean Young
-1 siblings, 0 replies; 5+ messages in thread
From: Sean Young @ 2020-08-08 18:14 UTC (permalink / raw)
To: kernel test robot
Cc: linux-media, kbuild-all, syzbot+ceef16277388d6f24898,
Hillf Danton
On Sat, Aug 08, 2020 at 10:42:00PM +0800, kernel test robot wrote:
> Hi Sean,
>
> I love your patch! Perhaps something to improve:
>
> [auto build test WARNING on linuxtv-media/master]
> [also build test WARNING on v5.8 next-20200807]
> [If your patch is applied to the wrong git tree, kindly drop us a note.
> And when submitting patch, we suggest to use '--base' as documented in
> https://git-scm.com/docs/git-format-patch]
>
> url: https://github.com/0day-ci/linux/commits/Sean-Young/media-rc-uevent-sysfs-file-races-with-rc_register_device/20200808-203329
> base: git://linuxtv.org/media_tree.git master
> config: x86_64-randconfig-m001-20200808 (attached as .config)
> compiler: gcc-9 (Debian 9.3.0-15) 9.3.0
>
> If you fix the issue, kindly add following tag as appropriate
> Reported-by: kernel test robot <lkp@intel.com>
>
> smatch warnings:
> drivers/media/rc/rc-main.c:1630 rc_dev_uevent() warn: inconsistent returns 'dev->lock'.
smatch is totally right here, there are code paths where the mutex is not
unlocked. Oops.
I'll send out a v2 shortly.
Sean
>
> vim +1630 drivers/media/rc/rc-main.c
>
> d8b4b5822f51e2 David Härdeman 2010-10-29 1603
> bc2a6c5719efd7 Mauro Carvalho Chehab 2010-11-09 1604 #define ADD_HOTPLUG_VAR(fmt, val...) \
> bc2a6c5719efd7 Mauro Carvalho Chehab 2010-11-09 1605 do { \
> bc2a6c5719efd7 Mauro Carvalho Chehab 2010-11-09 1606 int err = add_uevent_var(env, fmt, val); \
> bc2a6c5719efd7 Mauro Carvalho Chehab 2010-11-09 1607 if (err) \
> bc2a6c5719efd7 Mauro Carvalho Chehab 2010-11-09 1608 return err; \
> bc2a6c5719efd7 Mauro Carvalho Chehab 2010-11-09 1609 } while (0)
> bc2a6c5719efd7 Mauro Carvalho Chehab 2010-11-09 1610
> bc2a6c5719efd7 Mauro Carvalho Chehab 2010-11-09 1611 static int rc_dev_uevent(struct device *device, struct kobj_uevent_env *env)
> bc2a6c5719efd7 Mauro Carvalho Chehab 2010-11-09 1612 {
> d8b4b5822f51e2 David Härdeman 2010-10-29 1613 struct rc_dev *dev = to_rc_dev(device);
> bc2a6c5719efd7 Mauro Carvalho Chehab 2010-11-09 1614
> e4b7677d34e789 Sean Young 2020-08-08 1615 mutex_lock(&dev->lock);
> e4b7677d34e789 Sean Young 2020-08-08 1616 if (!dev->registered) {
> e4b7677d34e789 Sean Young 2020-08-08 1617 mutex_unlock(&dev->lock);
> e4b7677d34e789 Sean Young 2020-08-08 1618 return -ENODEV;
> e4b7677d34e789 Sean Young 2020-08-08 1619 }
> e4b7677d34e789 Sean Young 2020-08-08 1620
> b088ba658b3438 Mauro Carvalho Chehab 2010-11-17 1621 if (dev->rc_map.name)
> b088ba658b3438 Mauro Carvalho Chehab 2010-11-17 1622 ADD_HOTPLUG_VAR("NAME=%s", dev->rc_map.name);
> d8b4b5822f51e2 David Härdeman 2010-10-29 1623 if (dev->driver_name)
> d8b4b5822f51e2 David Härdeman 2010-10-29 1624 ADD_HOTPLUG_VAR("DRV_NAME=%s", dev->driver_name);
> b9f407e31c5073 Sean Young 2017-09-01 1625 if (dev->device_name)
> b9f407e31c5073 Sean Young 2017-09-01 1626 ADD_HOTPLUG_VAR("DEV_NAME=%s", dev->device_name);
> bc2a6c5719efd7 Mauro Carvalho Chehab 2010-11-09 1627
> e4b7677d34e789 Sean Young 2020-08-08 1628 mutex_unlock(&dev->lock);
> e4b7677d34e789 Sean Young 2020-08-08 1629
> bc2a6c5719efd7 Mauro Carvalho Chehab 2010-11-09 @1630 return 0;
> bc2a6c5719efd7 Mauro Carvalho Chehab 2010-11-09 1631 }
> bc2a6c5719efd7 Mauro Carvalho Chehab 2010-11-09 1632
>
> ---
> 0-DAY CI Kernel Test Service, Intel Corporation
> https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2020-08-08 18:14 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2020-08-08 12:31 [PATCH 1/2] media: rc: uevent sysfs file races with rc_register_device() Sean Young
2020-08-08 12:31 ` [PATCH 2/2] media: rc: do not access device via sysfs after rc_unregister_device() Sean Young
2020-08-08 14:42 ` [PATCH 1/2] media: rc: uevent sysfs file races with rc_register_device() kernel test robot
2020-08-08 14:42 ` kernel test robot
2020-08-08 18:14 ` Sean Young
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.