* fix broken locking in x25 ioctl error paths
@ 2013-06-28 15:14 Dave Jones
2013-06-28 15:19 ` Eric Dumazet
2013-06-28 16:13 ` Dave Jones
0 siblings, 2 replies; 5+ messages in thread
From: Dave Jones @ 2013-06-28 15:14 UTC (permalink / raw)
To: netdev
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>
diff --git a/net/x25/af_x25.c b/net/x25/af_x25.c
index 37ca969..2c1e633 100644
--- a/net/x25/af_x25.c
+++ b/net/x25/af_x25.c
@@ -1584,10 +1584,11 @@ out_cud_release:
rc = -EINVAL;
lock_sock(sk);
if (sk->sk_state != TCP_CLOSE)
- break;
+ goto out_callaccpt_release;
clear_bit(X25_ACCPT_APPRV_FLAG, &x25->flags);
- release_sock(sk);
rc = 0;
+out_callaccpt_release:
+ release_sock(sk);
break;
}
@@ -1595,14 +1596,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;
}
^ permalink raw reply related [flat|nested] 5+ messages in thread* Re: fix broken locking in x25 ioctl error paths
2013-06-28 15:14 fix broken locking in x25 ioctl error paths Dave Jones
@ 2013-06-28 15:19 ` Eric Dumazet
2013-06-28 15:36 ` Dave Jones
2013-06-28 16:13 ` Dave Jones
1 sibling, 1 reply; 5+ messages in thread
From: Eric Dumazet @ 2013-06-28 15:19 UTC (permalink / raw)
To: Dave Jones; +Cc: netdev
On Fri, 2013-06-28 at 11:14 -0400, Dave Jones wrote:
> 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>
>
> diff --git a/net/x25/af_x25.c b/net/x25/af_x25.c
> index 37ca969..2c1e633 100644
> --- a/net/x25/af_x25.c
> +++ b/net/x25/af_x25.c
> @@ -1584,10 +1584,11 @@ out_cud_release:
> rc = -EINVAL;
> lock_sock(sk);
> if (sk->sk_state != TCP_CLOSE)
> - break;
> + goto out_callaccpt_release;
> clear_bit(X25_ACCPT_APPRV_FLAG, &x25->flags);
> - release_sock(sk);
> rc = 0;
> +out_callaccpt_release:
> + release_sock(sk);
> break;
> }
Or :
lock_sock(sk);
if (sk->sk_state == TCP_CLOSE) {
clear_bit(X25_ACCPT_APPRV_FLAG, &x25->flags);
rc = 0;
}
release_sock(sk);
break;
^ permalink raw reply [flat|nested] 5+ messages in thread* Re: fix broken locking in x25 ioctl error paths
2013-06-28 15:19 ` Eric Dumazet
@ 2013-06-28 15:36 ` Dave Jones
0 siblings, 0 replies; 5+ messages in thread
From: Dave Jones @ 2013-06-28 15:36 UTC (permalink / raw)
To: Eric Dumazet; +Cc: netdev
On Fri, Jun 28, 2013 at 08:19:35AM -0700, Eric Dumazet wrote:
> On Fri, 2013-06-28 at 11:14 -0400, Dave Jones wrote:
> > 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>
> >
> > diff --git a/net/x25/af_x25.c b/net/x25/af_x25.c
> > index 37ca969..2c1e633 100644
> > --- a/net/x25/af_x25.c
> > +++ b/net/x25/af_x25.c
> > @@ -1584,10 +1584,11 @@ out_cud_release:
> > rc = -EINVAL;
> > lock_sock(sk);
> > if (sk->sk_state != TCP_CLOSE)
> > - break;
> > + goto out_callaccpt_release;
> > clear_bit(X25_ACCPT_APPRV_FLAG, &x25->flags);
> > - release_sock(sk);
> > rc = 0;
> > +out_callaccpt_release:
> > + release_sock(sk);
> > break;
> > }
>
> Or :
>
> lock_sock(sk);
> if (sk->sk_state == TCP_CLOSE) {
> clear_bit(X25_ACCPT_APPRV_FLAG, &x25->flags);
> rc = 0;
> }
> release_sock(sk);
> break;
I can do that if it's preferred. I just copied the same style
as the existing cases.
Dave
^ permalink raw reply [flat|nested] 5+ messages in thread
* fix broken locking in x25 ioctl error paths
2013-06-28 15:14 fix broken locking in x25 ioctl error paths Dave Jones
2013-06-28 15:19 ` Eric Dumazet
@ 2013-06-28 16:13 ` Dave Jones
2013-07-02 1:16 ` David Miller
1 sibling, 1 reply; 5+ messages in thread
From: Dave Jones @ 2013-06-28 16:13 UTC (permalink / raw)
To: netdev
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>
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;
}
^ permalink raw reply related [flat|nested] 5+ messages in thread* Re: fix broken locking in x25 ioctl error paths
2013-06-28 16:13 ` Dave Jones
@ 2013-07-02 1:16 ` David Miller
0 siblings, 0 replies; 5+ messages in thread
From: David Miller @ 2013-07-02 1:16 UTC (permalink / raw)
To: davej; +Cc: netdev
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!
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2013-07-02 1:16 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2013-06-28 15:14 fix broken locking in x25 ioctl error paths Dave Jones
2013-06-28 15:19 ` Eric Dumazet
2013-06-28 15:36 ` Dave Jones
2013-06-28 16:13 ` Dave Jones
2013-07-02 1:16 ` David Miller
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).