Patchwork fix broken locking in x25 ioctl error paths

login
register
mail settings
Submitter Dave Jones
Date June 28, 2013, 4:13 p.m.
Message ID <20130628161352.GA16505@redhat.com>
Download mbox | patch
Permalink /patch/255459/
State Accepted
Delegated to: David Miller
Headers show

Comments

Dave Jones - June 28, 2013, 4:13 p.m.
Two of the x25 ioctl cases have error paths that break out of the function without
unlocking the socket, leading to this warning:

--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
David Miller - July 2, 2013, 1:16 a.m.
From: Dave Jones <davej@redhat.com>
Date: Fri, 28 Jun 2013 12:13:52 -0400

> Two of the x25 ioctl cases have error paths that break out of the function without
> unlocking the socket, leading to this warning:
> 
> ================================================
> [ BUG: lock held when returning to user space! ]
> 3.10.0-rc7+ #36 Not tainted
> ------------------------------------------------
> trinity-child2/31407 is leaving the kernel with locks still held!
> 1 lock held by trinity-child2/31407:
>  #0:  (sk_lock-AF_X25){+.+.+.}, at: [<ffffffffa024b6da>] x25_ioctl+0x8a/0x740 [x25]
> 
> Signed-off-by: Dave Jones <davej@redhat.com>

Applied and queued up for -stable, thanks Dave.

In the future please prefix your patch subject lines with "$subsystem: ", for
this I rewrote it as:

x25: Fix broken locking in ioctl error paths.

Thanks!
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Patch

================================================
[ BUG: lock held when returning to user space! ]
3.10.0-rc7+ #36 Not tainted
------------------------------------------------
trinity-child2/31407 is leaving the kernel with locks still held!
1 lock held by trinity-child2/31407:
 #0:  (sk_lock-AF_X25){+.+.+.}, at: [<ffffffffa024b6da>] x25_ioctl+0x8a/0x740 [x25]

Signed-off-by: Dave Jones <davej@redhat.com>

diff --git a/net/x25/af_x25.c b/net/x25/af_x25.c
index 37ca969..22c88d2 100644
--- a/net/x25/af_x25.c
+++ b/net/x25/af_x25.c
@@ -1583,11 +1583,11 @@  out_cud_release:
 	case SIOCX25CALLACCPTAPPRV: {
 		rc = -EINVAL;
 		lock_sock(sk);
-		if (sk->sk_state != TCP_CLOSE)
-			break;
-		clear_bit(X25_ACCPT_APPRV_FLAG, &x25->flags);
+		if (sk->sk_state == TCP_CLOSE) {
+			clear_bit(X25_ACCPT_APPRV_FLAG, &x25->flags);
+			rc = 0;
+		}
 		release_sock(sk);
-		rc = 0;
 		break;
 	}
 
@@ -1595,14 +1595,15 @@  out_cud_release:
 		rc = -EINVAL;
 		lock_sock(sk);
 		if (sk->sk_state != TCP_ESTABLISHED)
-			break;
+			goto out_sendcallaccpt_release;
 		/* must call accptapprv above */
 		if (test_bit(X25_ACCPT_APPRV_FLAG, &x25->flags))
-			break;
+			goto out_sendcallaccpt_release;
 		x25_write_internal(sk, X25_CALL_ACCEPTED);
 		x25->state = X25_STATE_3;
-		release_sock(sk);
 		rc = 0;
+out_sendcallaccpt_release:
+		release_sock(sk);
 		break;
 	}