Re: [irda-users] [BUG] ircomm calling copy_from_user under lock

[Martin D. <li...@md...>]

On Fri, 31 Oct 2003, Stephen Hemminger wrote:

> Debug: sleeping function called from invalid context at include/asm/uaccess.h:498
> in_atomic():0, irqs_disabled():1
> Call Trace:
>  [<c01207a9>] __might_sleep+0x92/0x9f
>  [<fa438151>] ircomm_tty_write+0x161/0x3c4 [ircomm_tty]
> 
> Don't have an easy fix, because it can't just drop lock before copy means either:
> 	1) copy to local buffer first then to tx
> 	2) each write goes to a seperate skb


The patch below does the double-copy 1).

Martin

----------------------------

--- linux-2.6.0-test6-bk5/net/irda/ircomm/ircomm_tty.c	Sat Oct  4 11:44:01 2003
+++ v2.6.0-test6-bk5-md/net/irda/ircomm/ircomm_tty.c	Mon Oct  6 19:32:14 2003
@@ -666,6 +666,7 @@ static int ircomm_tty_write(struct tty_s
 			    const unsigned char *buf, int count)
 {
 	struct ircomm_tty_cb *self = (struct ircomm_tty_cb *) tty->driver_data;
+	unsigned char *buf2 = NULL;
 	unsigned long flags;
 	struct sk_buff *skb;
 	int tailroom = 0;
@@ -702,6 +703,18 @@ static int ircomm_tty_write(struct tty_s
 #endif
 	}
 
+	if (count < 1)
+		return 0;
+
+	/* additional copy to avoid copy_from_user under spinlock */
+	if (from_user) {
+		buf2 = kmalloc(count, GFP_KERNEL);
+		if (buf2 == NULL)
+			return -ENOMEM;
+		if (copy_from_user(buf2, buf, count))
+			return -EFAULT;
+	}
+
 	spin_lock_irqsave(&self->spinlock, flags);
 
 	/* Fetch current transmit buffer */
@@ -761,19 +774,19 @@ static int ircomm_tty_write(struct tty_s
 			 * change later on - Jean II */
 			self->tx_data_size = self->max_data_size;
 		}
-		
+
 		/* Copy data */
-		if (from_user)
-			copy_from_user(skb_put(skb,size), buf+len, size);
-		else
-			memcpy(skb_put(skb,size), buf+len, size);
-		
+		memcpy(skb_put(skb,size), ((from_user)?buf2:buf)+len, size);
+
 		count -= size;
 		len += size;
 	}
 
 	spin_unlock_irqrestore(&self->spinlock, flags);
 
+	if (from_user)
+		kfree(buf2);
+
 	/*     
 	 * Schedule a new thread which will transmit the frame as soon
 	 * as possible, but at a safe point in time. We do this so the
@@ -837,6 +850,7 @@ static void ircomm_tty_wait_until_sent(s
 {
 	struct ircomm_tty_cb *self = (struct ircomm_tty_cb *) tty->driver_data;
 	unsigned long orig_jiffies, poll_time;
+	unsigned long flags;
 	
 	IRDA_DEBUG(2, "%s()\n", __FUNCTION__ );
 
@@ -848,14 +862,18 @@ static void ircomm_tty_wait_until_sent(s
 	/* Set poll time to 200 ms */
 	poll_time = IRDA_MIN(timeout, MSECS_TO_JIFFIES(200));
 
+	spin_lock_irqsave(&self->spinlock, flags);
 	while (self->tx_skb && self->tx_skb->len) {
+		spin_unlock_irqrestore(&self->spinlock, flags);
 		current->state = TASK_INTERRUPTIBLE;
 		schedule_timeout(poll_time);
+		spin_lock_irqsave(&self->spinlock, flags);
 		if (signal_pending(current))
 			break;
 		if (timeout && time_after(jiffies, orig_jiffies + timeout))
 			break;
 	}
+	spin_unlock_irqrestore(&self->spinlock, flags);
 	current->state = TASK_RUNNING;
 }