[patch 0/3] [IUCV] fixes for net-2.6.25

[patch 0/3] [IUCV] fixes for net-2.6.25

[Ursula Braun]

-- 
Dave,

the following 3 patches are intended for 2.6.25 and contain:
- locking changes in iucv.c
- locking changes in af_iucv.c
- extra checkings for successful allocations in af_iucv.c
- secure handling of send_skb_q list in af_iucv.c

[patch 1/3] iucv: wrong irq-disabling locking at module load time

[Ursula Braun]

[-- Attachment #1: 707-iucv-irq.diff --]
[-- Type: text/plain, Size: 1096 bytes --]

From: Ursula Braun <braunu@de.ibm.com>

Linux may hang when running af_iucv socket programs concurrently
with a load of module netiucv. iucv_register() tries to take the
iucv_table_lock with spin_lock_irq. This conflicts with
iucv_connect() which has a need for an smp_call_function while
holding the iucv_table_lock.
Solution: use bh-disabling locking in iucv_register()

Signed-off-by: Ursula Braun <braunu@de.ibm.com>
---

 net/iucv/iucv.c |    4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff -urpN linux-2.6/net/iucv/iucv.c linux-2.6-patched/net/iucv/iucv.c
--- linux-2.6/net/iucv/iucv.c	2008-02-07 13:24:12.000000000 +0100
+++ linux-2.6-patched/net/iucv/iucv.c	2008-02-07 13:24:34.000000000 +0100
@@ -693,9 +693,9 @@ int iucv_register(struct iucv_handler *h
 		iucv_setmask_up();
 	INIT_LIST_HEAD(&handler->paths);
 
-	spin_lock_irq(&iucv_table_lock);
+	spin_lock_bh(&iucv_table_lock);
 	list_add_tail(&handler->list, &iucv_handler_list);
-	spin_unlock_irq(&iucv_table_lock);
+	spin_unlock_bh(&iucv_table_lock);
 	rc = 0;
 out_mutex:
 	mutex_unlock(&iucv_register_mutex);

-- 

[patch 2/3] af_iucv: broken send_skb_q results in endless loop

[Ursula Braun]

[-- Attachment #1: 713-afiucv.diff --]
[-- Type: text/plain, Size: 1472 bytes --]

From: Ursula Braun <braunu@de.ibm.com>

A race has been detected in iucv_callback_txdone().
skb_unlink has to be done inside the locked area.

In addition checkings for successful allocations are inserted.

Signed-off-by: Ursula Braun <braunu@de.ibm.com>
---

 net/iucv/af_iucv.c |    8 +++++++-
 1 file changed, 7 insertions(+), 1 deletion(-)

diff -urpN linux-2.6/net/iucv/af_iucv.c linux-2.6-patched/net/iucv/af_iucv.c
--- linux-2.6/net/iucv/af_iucv.c	2008-02-07 13:24:12.000000000 +0100
+++ linux-2.6-patched/net/iucv/af_iucv.c	2008-02-07 13:24:38.000000000 +0100
@@ -482,6 +482,10 @@ static int iucv_sock_connect(struct sock
 	/* Create path. */
 	iucv->path = iucv_path_alloc(IUCV_QUEUELEN_DEFAULT,
 				     IPRMDATA, GFP_KERNEL);
+	if (!iucv->path) {
+		err = -ENOMEM;
+		goto done;
+	}
 	err = iucv_path_connect(iucv->path, &af_iucv_handler,
 				sa->siucv_user_id, NULL, user_data, sk);
 	if (err) {
@@ -1094,6 +1098,8 @@ static void iucv_callback_rx(struct iucv
 
 save_message:
 	save_msg = kzalloc(sizeof(struct sock_msg_q), GFP_ATOMIC | GFP_DMA);
+	if (!save_msg)
+		return;
 	save_msg->path = path;
 	save_msg->msg = *msg;
 
@@ -1118,10 +1124,10 @@ static void iucv_callback_txdone(struct 
 			this = list_skb;
 			list_skb = list_skb->next;
 		} while (memcmp(&msg->tag, this->cb, 4) && list_skb);
+		__skb_unlink(this, list);
 
 		spin_unlock_irqrestore(&list->lock, flags);
 
-		skb_unlink(this, &iucv_sk(sk)->send_skb_q);
 		kfree_skb(this);
 	}
 

-- 

[patch 3/3] af_iucv: defensive programming of iucv_callback_txdone

[Ursula Braun]

[-- Attachment #1: 714-afiucv-txdone.diff --]
[-- Type: text/plain, Size: 1705 bytes --]

From: Ursula Braun <braunu@de.ibm.com>

The loop in iucv_callback_txdone presumes existence of an entry
with msg->tag in the send_skb_q list. In error cases this
assumption might be wrong and might cause an endless loop.
Loop is rewritten to guarantee loop end in case of missing
msg->tag entry in send_skb_q.

Signed-off-by: Ursula Braun <braunu@de.ibm.com>
---

 net/iucv/af_iucv.c |   21 ++++++++++++++-------
 1 file changed, 14 insertions(+), 7 deletions(-)

diff -urpN linux-2.6/net/iucv/af_iucv.c linux-2.6-patched/net/iucv/af_iucv.c
--- linux-2.6/net/iucv/af_iucv.c	2008-02-07 13:24:39.000000000 +0100
+++ linux-2.6-patched/net/iucv/af_iucv.c	2008-02-07 13:24:39.000000000 +0100
@@ -1112,24 +1112,31 @@ static void iucv_callback_txdone(struct 
 				 struct iucv_message *msg)
 {
 	struct sock *sk = path->private;
-	struct sk_buff *this;
+	struct sk_buff *this = NULL;
 	struct sk_buff_head *list = &iucv_sk(sk)->send_skb_q;
 	struct sk_buff *list_skb = list->next;
 	unsigned long flags;
 
-	if (list_skb) {
+	if (!skb_queue_empty(list)) {
 		spin_lock_irqsave(&list->lock, flags);
 
-		do {
-			this = list_skb;
+		while (list_skb != (struct sk_buff *)list) {
+			if (!memcmp(&msg->tag, list_skb->cb, 4)) {
+				this = list_skb;
+				break;
+			}
 			list_skb = list_skb->next;
-		} while (memcmp(&msg->tag, this->cb, 4) && list_skb);
-		__skb_unlink(this, list);
+		}
+		if (this)
+			__skb_unlink(this, list);
 
 		spin_unlock_irqrestore(&list->lock, flags);
 
-		kfree_skb(this);
+		if (this)
+			kfree_skb(this);
 	}
+	if (!this)
+		printk(KERN_ERR "AF_IUCV msg tag %u not found\n", msg->tag);
 
 	if (sk->sk_state == IUCV_CLOSING) {
 		if (skb_queue_empty(&iucv_sk(sk)->send_skb_q)) {

-- 

Re: [patch 0/3] [IUCV] fixes for net-2.6.25

[David Miller]

From: Ursula Braun <braunu@de.ibm.com>
Date: Thu, 07 Feb 2008 15:28:42 +0100

> -- 
> Dave,
> 
> the following 3 patches are intended for 2.6.25 and contain:
> - locking changes in iucv.c
> - locking changes in af_iucv.c
> - extra checkings for successful allocations in af_iucv.c
> - secure handling of send_skb_q list in af_iucv.c

Applied, thanks.