Bug#285563: kernel-source-2.4.27: drm locking fix missing in 2.4 kernels
On Mon, Dec 13, 2004 at 11:33:09PM -0700, dann frazier wrote:
> Package: kernel
> Version: 2.4.27-6
> Severity: important
>
> The fix for CAN-2004-1056, added in 2.6.8-11, also applies to 2.4 - however,
> I don't think it will compile, because 2.4 doesn't define the
> LOCK_TEST_WITH_RETURN() in drmP.h.
>
> from 2.6.8's changelog:
> * [SECURITY] Fix insufficient locking checks in DRM code;
> CAN-2004-1056 (Fabio M. Di Nitto).
>
> I've attached a backport of the patch from 2.6 w/ this macro missing.
>
> Sorry for being lazy and not fixing it myself - I don't have time to look at
> it now, but I also didn't want us to collectively forget about it.
Thanks,
I located the macro in 2.6, but it makes use of the filp element of
the drm lock, which doesn't exist. So I took example from gentoo
and identified the lock based on the pid.
I am building this now, and will likely put it up for testing tomorrow.
Current patches attached for referance.
--
Horms
# origin: Fabio M. Di Nitto
# cset: none
# inclusion: pending inclusion
# description: Fix insufficient locking checks in DRM code; CAN-2004-1056
# revision date: 2004-12-13
diff -urN kernel-source-2.4.27.orig/drivers/char/drm/i810_dma.c kernel-source-2.4.27/drivers/char/drm/i810_dma.c
--- kernel-source-2.4.27.orig/drivers/char/drm/i810_dma.c 2004-12-01 03:07:54.000000000 -0700
+++ kernel-source-2.4.27/drivers/char/drm/i810_dma.c 2004-12-13 22:18:50.404864367 -0700
@@ -952,10 +952,7 @@
drm_file_t *priv = filp->private_data;
drm_device_t *dev = priv->dev;
- if(!_DRM_LOCK_IS_HELD(dev->lock.hw_lock->lock)) {
- DRM_ERROR("i810_flush_ioctl called without lock held\n");
- return -EINVAL;
- }
+ LOCK_TEST_WITH_RETURN( dev, filp );
i810_flush_queue(dev);
return 0;
@@ -977,10 +974,7 @@
if (copy_from_user(&vertex, (drm_i810_vertex_t *)arg, sizeof(vertex)))
return -EFAULT;
- if(!_DRM_LOCK_IS_HELD(dev->lock.hw_lock->lock)) {
- DRM_ERROR("i810_dma_vertex called without lock held\n");
- return -EINVAL;
- }
+ LOCK_TEST_WITH_RETURN( dev, filp );
if(vertex.idx < 0 || vertex.idx > dma->buf_count) return -EINVAL;
@@ -1008,10 +1002,7 @@
if (copy_from_user(&clear, (drm_i810_clear_t *)arg, sizeof(clear)))
return -EFAULT;
- if(!_DRM_LOCK_IS_HELD(dev->lock.hw_lock->lock)) {
- DRM_ERROR("i810_clear_bufs called without lock held\n");
- return -EINVAL;
- }
+ LOCK_TEST_WITH_RETURN( dev, filp );
/* GH: Someone's doing nasty things... */
if (!dev->dev_private) {
@@ -1030,10 +1021,8 @@
drm_file_t *priv = filp->private_data;
drm_device_t *dev = priv->dev;
- if(!_DRM_LOCK_IS_HELD(dev->lock.hw_lock->lock)) {
- DRM_ERROR("i810_swap_buf called without lock held\n");
- return -EINVAL;
- }
+
+ LOCK_TEST_WITH_RETURN( dev, filp );
i810_dma_dispatch_swap( dev );
return 0;
@@ -1068,10 +1057,7 @@
if (copy_from_user(&d, (drm_i810_dma_t *)arg, sizeof(d)))
return -EFAULT;
- if(!_DRM_LOCK_IS_HELD(dev->lock.hw_lock->lock)) {
- DRM_ERROR("i810_dma called without lock held\n");
- return -EINVAL;
- }
+ LOCK_TEST_WITH_RETURN( dev, filp );
d.granted = 0;
@@ -1179,10 +1165,7 @@
return -EFAULT;
- if(!_DRM_LOCK_IS_HELD(dev->lock.hw_lock->lock)) {
- DRM_ERROR("i810_dma_mc called without lock held\n");
- return -EINVAL;
- }
+ LOCK_TEST_WITH_RETURN( dev, filp );
i810_dma_dispatch_mc(dev, dma->buflist[mc.idx], mc.used,
mc.last_render );
@@ -1227,10 +1210,7 @@
drm_device_t *dev = priv->dev;
drm_i810_private_t *dev_priv = (drm_i810_private_t *)dev->dev_private;
- if(!_DRM_LOCK_IS_HELD(dev->lock.hw_lock->lock)) {
- DRM_ERROR("i810_fstatus called without lock held\n");
- return -EINVAL;
- }
+ LOCK_TEST_WITH_RETURN( dev, filp );
return I810_READ(0x30008);
}
@@ -1241,10 +1221,7 @@
drm_device_t *dev = priv->dev;
drm_i810_private_t *dev_priv = (drm_i810_private_t *)dev->dev_private;
- if(!_DRM_LOCK_IS_HELD(dev->lock.hw_lock->lock)) {
- DRM_ERROR("i810_ov0_flip called without lock held\n");
- return -EINVAL;
- }
+ LOCK_TEST_WITH_RETURN( dev, filp );
//Tell the overlay to update
I810_WRITE(0x30000,dev_priv->overlay_physical | 0x80000000);
diff -urN kernel-source-2.4.27.orig/drivers/char/drm/i830_dma.c kernel-source-2.4.27/drivers/char/drm/i830_dma.c
--- kernel-source-2.4.27.orig/drivers/char/drm/i830_dma.c 2004-02-18 06:36:31.000000000 -0700
+++ kernel-source-2.4.27/drivers/char/drm/i830_dma.c 2004-12-13 22:15:53.955647778 -0700
@@ -1330,10 +1330,7 @@
drm_file_t *priv = filp->private_data;
drm_device_t *dev = priv->dev;
- if(!_DRM_LOCK_IS_HELD(dev->lock.hw_lock->lock)) {
- DRM_ERROR("i830_flush_ioctl called without lock held\n");
- return -EINVAL;
- }
+ LOCK_TEST_WITH_RETURN( dev, filp );
i830_flush_queue(dev);
return 0;
@@ -1354,10 +1351,7 @@
if (copy_from_user(&vertex, (drm_i830_vertex_t *)arg, sizeof(vertex)))
return -EFAULT;
- if(!_DRM_LOCK_IS_HELD(dev->lock.hw_lock->lock)) {
- DRM_ERROR("i830_dma_vertex called without lock held\n");
- return -EINVAL;
- }
+ LOCK_TEST_WITH_RETURN( dev, filp );
DRM_DEBUG("i830 dma vertex, idx %d used %d discard %d\n",
vertex.idx, vertex.used, vertex.discard);
@@ -1384,10 +1378,7 @@
if (copy_from_user(&clear, (drm_i830_clear_t *)arg, sizeof(clear)))
return -EFAULT;
- if(!_DRM_LOCK_IS_HELD(dev->lock.hw_lock->lock)) {
- DRM_ERROR("i830_clear_bufs called without lock held\n");
- return -EINVAL;
- }
+ LOCK_TEST_WITH_RETURN( dev, filp );
/* GH: Someone's doing nasty things... */
if (!dev->dev_private) {
@@ -1409,10 +1400,7 @@
DRM_DEBUG("i830_swap_bufs\n");
- if(!_DRM_LOCK_IS_HELD(dev->lock.hw_lock->lock)) {
- DRM_ERROR("i830_swap_buf called without lock held\n");
- return -EINVAL;
- }
+ LOCK_TEST_WITH_RETURN( dev, filp );
i830_dma_dispatch_swap( dev );
return 0;
@@ -1453,10 +1441,7 @@
DRM_DEBUG("%s\n", __FUNCTION__);
- if(!_DRM_LOCK_IS_HELD(dev->lock.hw_lock->lock)) {
- DRM_ERROR("i830_flip_buf called without lock held\n");
- return -EINVAL;
- }
+ LOCK_TEST_WITH_RETURN( dev, filp );
if (!dev_priv->page_flipping)
i830_do_init_pageflip( dev );
@@ -1495,10 +1480,7 @@
if (copy_from_user(&d, (drm_i830_dma_t *)arg, sizeof(d)))
return -EFAULT;
- if(!_DRM_LOCK_IS_HELD(dev->lock.hw_lock->lock)) {
- DRM_ERROR("i830_dma called without lock held\n");
- return -EINVAL;
- }
+ LOCK_TEST_WITH_RETURN( dev, filp );
d.granted = 0;
diff -urN kernel-source-2.4.27.orig/drivers/char/drm/i830_irq.c kernel-source-2.4.27/drivers/char/drm/i830_irq.c
--- kernel-source-2.4.27.orig/drivers/char/drm/i830_irq.c 2003-11-28 11:26:20.000000000 -0700
+++ kernel-source-2.4.27/drivers/char/drm/i830_irq.c 2004-12-13 22:15:53.965413403 -0700
@@ -130,10 +130,7 @@
drm_i830_irq_emit_t emit;
int result;
- if(!_DRM_LOCK_IS_HELD(dev->lock.hw_lock->lock)) {
- DRM_ERROR("i830_irq_emit called without lock held\n");
- return -EINVAL;
- }
+ LOCK_TEST_WITH_RETURN( dev, filp );
if ( !dev_priv ) {
DRM_ERROR( "%s called with no initialization\n", __FUNCTION__ );
# origin: torvalds (BitKeeper)
# cset: 1.971.58.39 (2.6) key=3e868ffdWSVmjpKhisQ9Y-t4YQuXkg (Modified)
# inclusion: upstream
# descrition: Update direct-rendering to current DRI CVS tree.
# revision date: Fri, 07 Jan 2005 16:14:09 +0900
#
# Note this patch has been modified. All components eccept for the addition
# of LOCK_TEST_WITH_RETURN drivers/char/drm/drmP.h have been removed.
# This portion has been updated to correlate locks using pid instead
# of filp as the latter does not exist in 2.4.27. This idea was taken from
# http://mirror.clarkson.edu/pub/distributions/gentoo-portage/sys-kernel/gentoo-sources/files/gentoo-sources-2.4.20-CAN-2004-1056.patch
# -- Horms 7th January 2004
#
# S rset: ChangeSet|1.971.58.38..1.971.58.39
# D rset: drivers/char/drm/drm_agpsupport.h|1.15..1.16
# D rset: drivers/char/drm/i830_dma.c|1.12..1.13
# D rset: drivers/char/drm/drm_proc.h|1.8..1.9
# D rset: drivers/char/drm/r128_cce.c|1.9..1.10
# D rset: drivers/char/drm/r128_state.c|1.10..1.11
# D rset: drivers/char/drm/radeon_cp.c|1.15..1.16
# D rset: drivers/char/drm/drm_bufs.h|1.9..1.10
# D rset: drivers/char/drm/drm_ioctl.h|1.8..1.9
# D rset: drivers/char/drm/i830.h|1.5..1.6
# D rset: drivers/char/drm/i810.h|1.5..1.6
# D rset: drivers/char/drm/i810_dma.c|1.20..1.21
# D rset: drivers/char/drm/i830_drm.h|1.5..1.6
# D rset: drivers/char/drm/mga_state.c|1.12..1.13
# D rset: drivers/char/drm/gamma_drv.h|1.5..1.6
# D rset: drivers/char/drm/radeon_drv.h|1.17..1.18
# D rset: drivers/char/drm/drm_fops.h|1.8..1.9
# D rset: drivers/char/drm/drm_dma.h|1.10..1.11
# D rset: drivers/char/drm/radeon_drm.h|1.11..1.12
# D rset: drivers/char/drm/drm_lock.h|1.6..1.7
# D rset: drivers/char/drm/sis_mm.c|1.4..1.5
# D rset: drivers/char/drm/i830_drv.h|1.6..1.7
# I rset: drivers/char/drm/drmP.h|1.17..1.18
# D rset: drivers/char/drm/drm_drv.h|1.14..1.15
# D rset: BitKeeper/deleted/.del-drm_lists.h~8e5aa1ea5d68c6c1|1.6..1.7
# D rset: drivers/char/drm/radeon_state.c|1.18..1.19
# D rset: drivers/char/drm/Kconfig|1.2..1.3
# D rset: drivers/char/drm/radeon_mem.c|1.5..1.6
# D rset: drivers/char/drm/mga_dma.c|1.10..1.11
# D rset: drivers/char/drm/mga_drv.h|1.11..1.12
# D rset: drivers/char/drm/radeon.h|1.9..1.10
# D rset: drivers/char/drm/drm_os_linux.h|1.7..1.8
# D rset: drivers/char/drm/gamma_dma.c|1.8..1.9
# D rset: drivers/char/drm/radeon_irq.c|1.7..1.8
# D rset: drivers/char/drm/i810_drv.h|1.7..1.8
# D rset: drivers/char/drm/Makefile|1.15..1.16
# D rset: drivers/char/drm/sis.h|1.4..1.5
# D rset: drivers/char/drm/r128_drv.h|1.12..1.13
# D rset: drivers/char/drm/i830_irq.c|1.0..1.1
#
# Key:
# S: Skipped ChangeSet file only
# O: Original Followed by Updated
# U: Updated Included with updated range of versions
# I: Included Included verbatim
# E: Excluded Excluded on request from user
# D: Deleted Manually deleted by subsequent user edit
#
# This is a BitKeeper generated diff -Nru style patch.
#
# ChangeSet
# 2003/03/29 22:34:37-08:00 torvalds@home.transmeta.com
# Update direct-rendering to current DRI CVS tree.
#
# This adds support for i830 interrupt handling, and new improved
# lock context keying. See per-file comments for more detail, as this
# commit sadly mixes up a few different things (that's what you get
# for not tracking the changes at a fine enough granularity).
#
# drivers/char/drm/drmP.h
# 2003/03/29 22:34:33-08:00 torvalds@home.transmeta.com +20 -5
# Make the DRI locking use "struct file" as the fundamental identity of
# a direct-rendering context. This fixes various leaks, and makes sure
# that we don't get confused by the file descriptor being released.
#
--- 1.17/drivers/char/drm/drmP.h 2002-12-19 02:56:28 +09:00
+++ 1.18/drivers/char/drm/drmP.h 2003-03-30 15:34:33 +09:00
@@ -268,6 +269,17 @@
(_map) = (_dev)->context_sareas[_ctx]; \
} while(0)
+#define LOCK_TEST_WITH_RETURN( dev, filp ) \
+do { \
+ if ( !_DRM_LOCK_IS_HELD( dev->lock.hw_lock->lock ) || \
+ dev->lock.pid != current->pid ) { \
+ DRM_ERROR( "%s called without lock held\n", \
+ __FUNCTION__ ); \
+ return -EINVAL; \
+ } \
+} while (0)
+
+
typedef int drm_ioctl_t( struct inode *inode, struct file *filp,
unsigned int cmd, unsigned long arg );
Reply to: