Bug 2748

Summary: Missing locks in video1394_ioctl
Product: Drivers Reporter: Herbert Xu (herbert)
Component: IEEE1394Assignee: Stefan Richter (stefanr)
Status: CLOSED CODE_FIX    
Severity: normal CC: bonnaud
Priority: P2    
Hardware: i386   
OS: Linux   
Kernel Version: 2.6.6 Subsystem:
Regression: --- Bisected commit-id:

Description Herbert Xu 2004-05-22 04:01:00 UTC 
This bug was reported at http://bugs.debian.org/248996.

The last few messages are:

video1394_0: Iso receive DMA: 8 buffers of size 614400 allocated for a frame sis
video1394_0: iso context 1 listen on channel 0
video1394_0: Waking up iso dma ctx=1
video1394_0: Buffer 3 is already used
video1394_0: Iso context 1 stop talking on channel 0
NMI Watchdog detected LOCKUP on CPU1, eip c02920f0, registers:
CPU:    1
EIP:    0060:[<c02920f0>]    Not tainted
EFLAGS: 00200086   (2.6.6-1-686-smp)
EIP is at .text.lock.sched+0x34/0xa4
eax: 00000000   ebx: f3106000   ecx: c1813be0   edx: f3106000
esi: f3c49edc   edi: 00200286   ebp: f3107ed0   esp: f3107ea4
ds: 007b   es: 007b   ss: 0068
Process Simulation_trac (pid: 2218, threadinfo=f3106000 task=f77ba0d0)
Stack: 00000000 f77ba0d0 c011bb60 f3c49ee0 f3c49ee0 c01b48c2 f3107f44 bf5ffa84
       f3106000 f3c49e48 f3106000 00200297 f8aafbcd f77f7a20 00000001 00000000
       f77ba0d0 c011bb60 00000000 00000000 f77515b8 f77ba5f4 f3107fc4 f3c49edc
Call Trace:
 [<c011bb60>] default_wake_function+0x0/0x20
 [<c01b48c2>] copy_from_user+0x42/0x70
 [<f8aafbcd>] video1394_ioctl+0xacd/0x1070 [video1394]
 [<c011bb60>] default_wake_function+0x0/0x20
 [<c010dc66>] convert_fxsr_from_user+0x26/0xf0
 [<c01e4f8e>] tty_write+0x19e/0x2f0
 [<c01eab80>] write_chan+0x0/0x230
 [<c015d7b7>] vfs_write+0x107/0x160
 [<c0171c08>] sys_ioctl+0x148/0x2d0
 [<c0102313>] huft_build+0x1d3/0x540
 [<c010621b>] syscall_call+0x7/0xb
 [<c0102313>] huft_build+0x1d3/0x540
                                                                               
                                                                               
                  
Code: 80 3e 00 7e f9 e9 9e fb ff ff f3 90 80 3e 00 7e f9 e9 11 fc
console shuts up ...

So it looks like someone did a WAIT/POLL BUFFER which sleeps without holding a
lock on d, and then someone did a UNLISTEN/UNTALK CHANNEL, which frees d...
Comment 1 Stefan Richter 2006-08-01 12:35:21 UTC 
This may have been fixed (although inelegantly using the big kernel lock) by
patch "Convert video1394 driver to compat_ioctl" which went into linux1394.org's
repo in r1253 on 2005-02-02.

--- trunk/video1394.c	(revision 1252)
+++ trunk/video1394.c	(revision 1253)
@@ -710,8 +710,8 @@
 	reg_write(ohci, OHCI1394_IsoXmitIntMaskSet, 1<<d->ctx);
 }
 
-static int video1394_ioctl(struct inode *inode, struct file *file,
-			   unsigned int cmd, unsigned long arg)
+static int __video1394_ioctl(struct file *file,
+			     unsigned int cmd, unsigned long arg)
 {
 	struct file_ctx *ctx = (struct file_ctx *)file->private_data;
 	struct ti_ohci *ohci = ctx->ohci;
@@ -1170,6 +1170,15 @@
 	}
 }
 
+static long video1394_ioctl(struct file *file, unsigned int cmd, unsigned long arg)
+{
+	int err;
+	lock_kernel();
+	err = __video1394_ioctl(file, cmd, arg);
+	unlock_kernel();
+	return err;
+}
+
 /*
  *	This maps the vmalloced and reserved buffer to user space.
  *
[...rest of patch omitted...]
Comment 2 Stefan Richter 2006-08-01 12:41:37 UTC 
I assume this case is closed. Please reopen if I'm wrong.