* Re: [PATCH v4 3/5] drm: lock drm_global_mutex earlier in the ioctl handler
@ 2021-08-20 22:42 kernel test robot
0 siblings, 0 replies; 3+ messages in thread
From: kernel test robot @ 2021-08-20 22:42 UTC (permalink / raw)
To: kbuild
[-- Attachment #1: Type: text/plain, Size: 8594 bytes --]
CC: kbuild-all(a)lists.01.org
In-Reply-To: <20210820100251.448346-4-desmondcheongzx@gmail.com>
References: <20210820100251.448346-4-desmondcheongzx@gmail.com>
TO: Desmond Cheong Zhi Xi <desmondcheongzx@gmail.com>
TO: maarten.lankhorst(a)linux.intel.com
TO: mripard(a)kernel.org
TO: tzimmermann(a)suse.de
TO: airlied(a)linux.ie
TO: daniel(a)ffwll.ch
TO: sumit.semwal(a)linaro.org
TO: christian.koenig(a)amd.com
CC: Desmond Cheong Zhi Xi <desmondcheongzx@gmail.com>
CC: dri-devel(a)lists.freedesktop.org
CC: linux-kernel(a)vger.kernel.org
Hi Desmond,
Thank you for the patch! Perhaps something to improve:
[auto build test WARNING on next-20210819]
[cannot apply to linus/master v5.14-rc6 v5.14-rc5 v5.14-rc4 v5.14-rc6]
[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/Desmond-Cheong-Zhi-Xi/drm-update-locking-for-modesetting/20210820-180518
base: 33e65b1f975cd2814fc0ea9617250fc4c1d7a553
:::::: branch date: 13 hours ago
:::::: commit date: 13 hours ago
config: x86_64-randconfig-c022-20210821 (attached as .config)
compiler: gcc-9 (Debian 9.3.0-22) 9.3.0
If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>
Reported-by: Julia Lawall <julia.lawall@lip6.fr>
cocci warnings: (new ones prefixed by >>)
>> drivers/gpu/drm/drm_ioctl.c:801:1-7: preceding lock on line 790
vim +801 drivers/gpu/drm/drm_ioctl.c
78238757ebab54 Daniel Vetter 2014-09-10 729
2640981f360040 Daniel Vetter 2017-04-04 730 /**
2640981f360040 Daniel Vetter 2017-04-04 731 * DOC: driver specific ioctls
2640981f360040 Daniel Vetter 2017-04-04 732 *
2640981f360040 Daniel Vetter 2017-04-04 733 * First things first, driver private IOCTLs should only be needed for drivers
2640981f360040 Daniel Vetter 2017-04-04 734 * supporting rendering. Kernel modesetting is all standardized, and extended
2640981f360040 Daniel Vetter 2017-04-04 735 * through properties. There are a few exceptions in some existing drivers,
2640981f360040 Daniel Vetter 2017-04-04 736 * which define IOCTL for use by the display DRM master, but they all predate
2640981f360040 Daniel Vetter 2017-04-04 737 * properties.
2640981f360040 Daniel Vetter 2017-04-04 738 *
2640981f360040 Daniel Vetter 2017-04-04 739 * Now if you do have a render driver you always have to support it through
2640981f360040 Daniel Vetter 2017-04-04 740 * driver private properties. There's a few steps needed to wire all the things
2640981f360040 Daniel Vetter 2017-04-04 741 * up.
2640981f360040 Daniel Vetter 2017-04-04 742 *
2640981f360040 Daniel Vetter 2017-04-04 743 * First you need to define the structure for your IOCTL in your driver private
2640981f360040 Daniel Vetter 2017-04-04 744 * UAPI header in ``include/uapi/drm/my_driver_drm.h``::
2640981f360040 Daniel Vetter 2017-04-04 745 *
2640981f360040 Daniel Vetter 2017-04-04 746 * struct my_driver_operation {
2640981f360040 Daniel Vetter 2017-04-04 747 * u32 some_thing;
2640981f360040 Daniel Vetter 2017-04-04 748 * u32 another_thing;
2640981f360040 Daniel Vetter 2017-04-04 749 * };
2640981f360040 Daniel Vetter 2017-04-04 750 *
2640981f360040 Daniel Vetter 2017-04-04 751 * Please make sure that you follow all the best practices from
72ef5e52b3f74c Mauro Carvalho Chehab 2020-04-14 752 * ``Documentation/process/botching-up-ioctls.rst``. Note that drm_ioctl()
2640981f360040 Daniel Vetter 2017-04-04 753 * automatically zero-extends structures, hence make sure you can add more stuff
2640981f360040 Daniel Vetter 2017-04-04 754 * at the end, i.e. don't put a variable sized array there.
2640981f360040 Daniel Vetter 2017-04-04 755 *
2640981f360040 Daniel Vetter 2017-04-04 756 * Then you need to define your IOCTL number, using one of DRM_IO(), DRM_IOR(),
2640981f360040 Daniel Vetter 2017-04-04 757 * DRM_IOW() or DRM_IOWR(). It must start with the DRM_IOCTL\_ prefix::
2640981f360040 Daniel Vetter 2017-04-04 758 *
2640981f360040 Daniel Vetter 2017-04-04 759 * ##define DRM_IOCTL_MY_DRIVER_OPERATION \
2640981f360040 Daniel Vetter 2017-04-04 760 * DRM_IOW(DRM_COMMAND_BASE, struct my_driver_operation)
2640981f360040 Daniel Vetter 2017-04-04 761 *
2640981f360040 Daniel Vetter 2017-04-04 762 * DRM driver private IOCTL must be in the range from DRM_COMMAND_BASE to
2640981f360040 Daniel Vetter 2017-04-04 763 * DRM_COMMAND_END. Finally you need an array of &struct drm_ioctl_desc to wire
bb2eaba6458ace Daniel Vetter 2017-05-31 764 * up the handlers and set the access rights::
2640981f360040 Daniel Vetter 2017-04-04 765 *
2640981f360040 Daniel Vetter 2017-04-04 766 * static const struct drm_ioctl_desc my_driver_ioctls[] = {
2640981f360040 Daniel Vetter 2017-04-04 767 * DRM_IOCTL_DEF_DRV(MY_DRIVER_OPERATION, my_driver_operation,
2640981f360040 Daniel Vetter 2017-04-04 768 * DRM_AUTH|DRM_RENDER_ALLOW),
2640981f360040 Daniel Vetter 2017-04-04 769 * };
2640981f360040 Daniel Vetter 2017-04-04 770 *
2640981f360040 Daniel Vetter 2017-04-04 771 * And then assign this to the &drm_driver.ioctls field in your driver
2640981f360040 Daniel Vetter 2017-04-04 772 * structure.
bb2eaba6458ace Daniel Vetter 2017-05-31 773 *
bb2eaba6458ace Daniel Vetter 2017-05-31 774 * See the separate chapter on :ref:`file operations<drm_driver_fops>` for how
bb2eaba6458ace Daniel Vetter 2017-05-31 775 * the driver-specific IOCTLs are wired up.
2640981f360040 Daniel Vetter 2017-04-04 776 */
2640981f360040 Daniel Vetter 2017-04-04 777
7f0dfc1625dbaa Al Viro 2017-05-24 778 long drm_ioctl_kernel(struct file *file, drm_ioctl_t *func, void *kdata,
7f0dfc1625dbaa Al Viro 2017-05-24 779 u32 flags)
7f0dfc1625dbaa Al Viro 2017-05-24 780 {
7f0dfc1625dbaa Al Viro 2017-05-24 781 struct drm_file *file_priv = file->private_data;
7f0dfc1625dbaa Al Viro 2017-05-24 782 struct drm_device *dev = file_priv->minor->dev;
7f0dfc1625dbaa Al Viro 2017-05-24 783 int retcode;
7f0dfc1625dbaa Al Viro 2017-05-24 784
c07dcd61a0e57c Daniel Vetter 2017-08-02 785 if (drm_dev_is_unplugged(dev))
7f0dfc1625dbaa Al Viro 2017-05-24 786 return -ENODEV;
7f0dfc1625dbaa Al Viro 2017-05-24 787
d36e4e473b30d7 Desmond Cheong Zhi Xi 2021-08-20 788 /* Enforce sane locking for modern driver ioctls. */
d36e4e473b30d7 Desmond Cheong Zhi Xi 2021-08-20 789 if (unlikely(drm_core_check_feature(dev, DRIVER_LEGACY)) && !(flags & DRM_UNLOCKED))
d36e4e473b30d7 Desmond Cheong Zhi Xi 2021-08-20 @790 mutex_lock(&drm_global_mutex);
d36e4e473b30d7 Desmond Cheong Zhi Xi 2021-08-20 791
7f0dfc1625dbaa Al Viro 2017-05-24 792 retcode = drm_ioctl_permit(flags, file_priv);
7f0dfc1625dbaa Al Viro 2017-05-24 793 if (unlikely(retcode))
d36e4e473b30d7 Desmond Cheong Zhi Xi 2021-08-20 794 goto out;
7f0dfc1625dbaa Al Viro 2017-05-24 795
7f0dfc1625dbaa Al Viro 2017-05-24 796 retcode = func(dev, kdata, file_priv);
d36e4e473b30d7 Desmond Cheong Zhi Xi 2021-08-20 797
d36e4e473b30d7 Desmond Cheong Zhi Xi 2021-08-20 798 out:
d36e4e473b30d7 Desmond Cheong Zhi Xi 2021-08-20 799 if (unlikely(drm_core_check_feature(dev, DRIVER_LEGACY)) && !(flags & DRM_UNLOCKED))
7f0dfc1625dbaa Al Viro 2017-05-24 800 mutex_unlock(&drm_global_mutex);
7f0dfc1625dbaa Al Viro 2017-05-24 @801 return retcode;
7f0dfc1625dbaa Al Viro 2017-05-24 802 }
aeba03903063e9 Al Viro 2017-06-03 803 EXPORT_SYMBOL(drm_ioctl_kernel);
7f0dfc1625dbaa Al Viro 2017-05-24 804
---
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: 35205 bytes --]
^ permalink raw reply [flat|nested] 3+ messages in thread
* [PATCH v4 3/5] drm: lock drm_global_mutex earlier in the ioctl handler
2021-08-20 10:02 [PATCH v4 0/5] drm: update locking for modesetting Desmond Cheong Zhi Xi
@ 2021-08-20 10:02 ` Desmond Cheong Zhi Xi
0 siblings, 0 replies; 3+ messages in thread
From: Desmond Cheong Zhi Xi @ 2021-08-20 10:02 UTC (permalink / raw)
To: maarten.lankhorst, mripard, tzimmermann, airlied, daniel,
sumit.semwal, christian.koenig
Cc: Desmond Cheong Zhi Xi, dri-devel, linux-kernel, intel-gfx,
linux-media, linaro-mm-sig, skhan, gregkh, linux-kernel-mentees
In a future patch, a read lock on drm_device.master_rwsem is
held in the ioctl handler before the check for ioctl
permissions. However, this inverts the lock hierarchy of
drm_global_mutex --> master_rwsem.
To avoid this, we do some prep work to grab the drm_global_mutex
before checking for ioctl permissions.
Signed-off-by: Desmond Cheong Zhi Xi <desmondcheongzx@gmail.com>
---
drivers/gpu/drm/drm_ioctl.c | 18 +++++++++---------
1 file changed, 9 insertions(+), 9 deletions(-)
diff --git a/drivers/gpu/drm/drm_ioctl.c b/drivers/gpu/drm/drm_ioctl.c
index d25713b09b80..158629d88319 100644
--- a/drivers/gpu/drm/drm_ioctl.c
+++ b/drivers/gpu/drm/drm_ioctl.c
@@ -772,19 +772,19 @@ long drm_ioctl_kernel(struct file *file, drm_ioctl_t *func, void *kdata,
if (drm_dev_is_unplugged(dev))
return -ENODEV;
+ /* Enforce sane locking for modern driver ioctls. */
+ if (unlikely(drm_core_check_feature(dev, DRIVER_LEGACY)) && !(flags & DRM_UNLOCKED))
+ mutex_lock(&drm_global_mutex);
+
retcode = drm_ioctl_permit(flags, file_priv);
if (unlikely(retcode))
- return retcode;
+ goto out;
- /* Enforce sane locking for modern driver ioctls. */
- if (likely(!drm_core_check_feature(dev, DRIVER_LEGACY)) ||
- (flags & DRM_UNLOCKED))
- retcode = func(dev, kdata, file_priv);
- else {
- mutex_lock(&drm_global_mutex);
- retcode = func(dev, kdata, file_priv);
+ retcode = func(dev, kdata, file_priv);
+
+out:
+ if (unlikely(drm_core_check_feature(dev, DRIVER_LEGACY)) && !(flags & DRM_UNLOCKED))
mutex_unlock(&drm_global_mutex);
- }
return retcode;
}
EXPORT_SYMBOL(drm_ioctl_kernel);
--
2.25.1
^ permalink raw reply related [flat|nested] 3+ messages in thread
* [PATCH v4 3/5] drm: lock drm_global_mutex earlier in the ioctl handler
@ 2021-08-20 10:02 ` Desmond Cheong Zhi Xi
0 siblings, 0 replies; 3+ messages in thread
From: Desmond Cheong Zhi Xi @ 2021-08-20 10:02 UTC (permalink / raw)
To: maarten.lankhorst, mripard, tzimmermann, airlied, daniel,
sumit.semwal, christian.koenig
Cc: intel-gfx, linux-kernel, dri-devel, linaro-mm-sig,
Desmond Cheong Zhi Xi, linux-kernel-mentees, linux-media
In a future patch, a read lock on drm_device.master_rwsem is
held in the ioctl handler before the check for ioctl
permissions. However, this inverts the lock hierarchy of
drm_global_mutex --> master_rwsem.
To avoid this, we do some prep work to grab the drm_global_mutex
before checking for ioctl permissions.
Signed-off-by: Desmond Cheong Zhi Xi <desmondcheongzx@gmail.com>
---
drivers/gpu/drm/drm_ioctl.c | 18 +++++++++---------
1 file changed, 9 insertions(+), 9 deletions(-)
diff --git a/drivers/gpu/drm/drm_ioctl.c b/drivers/gpu/drm/drm_ioctl.c
index d25713b09b80..158629d88319 100644
--- a/drivers/gpu/drm/drm_ioctl.c
+++ b/drivers/gpu/drm/drm_ioctl.c
@@ -772,19 +772,19 @@ long drm_ioctl_kernel(struct file *file, drm_ioctl_t *func, void *kdata,
if (drm_dev_is_unplugged(dev))
return -ENODEV;
+ /* Enforce sane locking for modern driver ioctls. */
+ if (unlikely(drm_core_check_feature(dev, DRIVER_LEGACY)) && !(flags & DRM_UNLOCKED))
+ mutex_lock(&drm_global_mutex);
+
retcode = drm_ioctl_permit(flags, file_priv);
if (unlikely(retcode))
- return retcode;
+ goto out;
- /* Enforce sane locking for modern driver ioctls. */
- if (likely(!drm_core_check_feature(dev, DRIVER_LEGACY)) ||
- (flags & DRM_UNLOCKED))
- retcode = func(dev, kdata, file_priv);
- else {
- mutex_lock(&drm_global_mutex);
- retcode = func(dev, kdata, file_priv);
+ retcode = func(dev, kdata, file_priv);
+
+out:
+ if (unlikely(drm_core_check_feature(dev, DRIVER_LEGACY)) && !(flags & DRM_UNLOCKED))
mutex_unlock(&drm_global_mutex);
- }
return retcode;
}
EXPORT_SYMBOL(drm_ioctl_kernel);
--
2.25.1
_______________________________________________
Linux-kernel-mentees mailing list
Linux-kernel-mentees@lists.linuxfoundation.org
https://lists.linuxfoundation.org/mailman/listinfo/linux-kernel-mentees
^ permalink raw reply related [flat|nested] 3+ messages in thread
end of thread, other threads:[~2021-08-20 22:42 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-08-20 22:42 [PATCH v4 3/5] drm: lock drm_global_mutex earlier in the ioctl handler kernel test robot
-- strict thread matches above, loose matches on Subject: below --
2021-08-20 10:02 [PATCH v4 0/5] drm: update locking for modesetting Desmond Cheong Zhi Xi
2021-08-20 10:02 ` [PATCH v4 3/5] drm: lock drm_global_mutex earlier in the ioctl handler Desmond Cheong Zhi Xi
2021-08-20 10:02 ` Desmond Cheong Zhi Xi
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.