diff mbox

SUNRPC: Set alloc_slot for backchannel tcp ops

Message ID 1348508341-19642-1-git-send-email-bjschuma@netapp.com (mailing list archive)
State New, archived
Headers show

Commit Message

Bryan Schumaker Sept. 24, 2012, 5:39 p.m. UTC
From: Bryan Schumaker <bjschuma@netapp.com>

f39c1bfb5a03e2d255451bff05be0d7255298fa4 (SUNRPC: Fix a UDP transport
regression) introduced the "alloc_slot" function for xprt operations,
but never created one for the backchannel operations.  This patch fixes
a null pointer dereference when mounting NFS over v4.1.

Call Trace:
 [<ffffffffa0207957>] ? xprt_reserve+0x47/0x50 [sunrpc]
 [<ffffffffa02023a4>] call_reserve+0x34/0x60 [sunrpc]
 [<ffffffffa020e280>] __rpc_execute+0x90/0x400 [sunrpc]
 [<ffffffffa020e61a>] rpc_async_schedule+0x2a/0x40 [sunrpc]
 [<ffffffff81073589>] process_one_work+0x139/0x500
 [<ffffffff81070e70>] ? alloc_worker+0x70/0x70
 [<ffffffffa020e5f0>] ? __rpc_execute+0x400/0x400 [sunrpc]
 [<ffffffff81073d1e>] worker_thread+0x15e/0x460
 [<ffffffff8145c839>] ? preempt_schedule+0x49/0x70
 [<ffffffff81073bc0>] ? rescuer_thread+0x230/0x230
 [<ffffffff81079603>] kthread+0x93/0xa0
 [<ffffffff81465d04>] kernel_thread_helper+0x4/0x10
 [<ffffffff81079570>] ? kthread_freezable_should_stop+0x70/0x70
 [<ffffffff81465d00>] ? gs_change+0x13/0x13

Signed-off-by: Bryan Schumaker <bjschuma@netapp.com>
---
 net/sunrpc/xprtsock.c | 1 +
 1 file changed, 1 insertion(+)

Comments

J. Bruce Fields Sept. 24, 2012, 5:42 p.m. UTC | #1
On Mon, Sep 24, 2012 at 01:39:01PM -0400, bjschuma@netapp.com wrote:
> From: Bryan Schumaker <bjschuma@netapp.com>
> 
> f39c1bfb5a03e2d255451bff05be0d7255298fa4 (SUNRPC: Fix a UDP transport
> regression) introduced the "alloc_slot" function for xprt operations,
> but never created one for the backchannel operations.  This patch fixes
> a null pointer dereference when mounting NFS over v4.1.

Thanks, I just rebased some of my work to 3.6 and ran across that!  It
crashes the 4.1 server very quickly....

--b.

> 
> Call Trace:
>  [<ffffffffa0207957>] ? xprt_reserve+0x47/0x50 [sunrpc]
>  [<ffffffffa02023a4>] call_reserve+0x34/0x60 [sunrpc]
>  [<ffffffffa020e280>] __rpc_execute+0x90/0x400 [sunrpc]
>  [<ffffffffa020e61a>] rpc_async_schedule+0x2a/0x40 [sunrpc]
>  [<ffffffff81073589>] process_one_work+0x139/0x500
>  [<ffffffff81070e70>] ? alloc_worker+0x70/0x70
>  [<ffffffffa020e5f0>] ? __rpc_execute+0x400/0x400 [sunrpc]
>  [<ffffffff81073d1e>] worker_thread+0x15e/0x460
>  [<ffffffff8145c839>] ? preempt_schedule+0x49/0x70
>  [<ffffffff81073bc0>] ? rescuer_thread+0x230/0x230
>  [<ffffffff81079603>] kthread+0x93/0xa0
>  [<ffffffff81465d04>] kernel_thread_helper+0x4/0x10
>  [<ffffffff81079570>] ? kthread_freezable_should_stop+0x70/0x70
>  [<ffffffff81465d00>] ? gs_change+0x13/0x13
> 
> Signed-off-by: Bryan Schumaker <bjschuma@netapp.com>
> ---
>  net/sunrpc/xprtsock.c | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/net/sunrpc/xprtsock.c b/net/sunrpc/xprtsock.c
> index 86b7777..aaaadfb 100644
> --- a/net/sunrpc/xprtsock.c
> +++ b/net/sunrpc/xprtsock.c
> @@ -2521,6 +2521,7 @@ static struct rpc_xprt_ops xs_tcp_ops = {
>  static struct rpc_xprt_ops bc_tcp_ops = {
>  	.reserve_xprt		= xprt_reserve_xprt,
>  	.release_xprt		= xprt_release_xprt,
> +	.alloc_slot		= xprt_alloc_slot,
>  	.rpcbind		= xs_local_rpcbind,
>  	.buf_alloc		= bc_malloc,
>  	.buf_free		= bc_free,
> -- 
> 1.7.12.1
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-nfs" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
--
To unsubscribe from this list: send the line "unsubscribe linux-nfs" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Bryan Schumaker Sept. 24, 2012, 5:52 p.m. UTC | #2
On 09/24/2012 01:42 PM, J. Bruce Fields wrote:
> On Mon, Sep 24, 2012 at 01:39:01PM -0400, bjschuma@netapp.com wrote:
>> From: Bryan Schumaker <bjschuma@netapp.com>
>>
>> f39c1bfb5a03e2d255451bff05be0d7255298fa4 (SUNRPC: Fix a UDP transport
>> regression) introduced the "alloc_slot" function for xprt operations,
>> but never created one for the backchannel operations.  This patch fixes
>> a null pointer dereference when mounting NFS over v4.1.
> 
> Thanks, I just rebased some of my work to 3.6 and ran across that!  It
> crashes the 4.1 server very quickly....

That sounds like my story.  It got my peer-to-peer server right away, too.

- Bryan

> 
> --b.
> 
>>
>> Call Trace:
>>  [<ffffffffa0207957>] ? xprt_reserve+0x47/0x50 [sunrpc]
>>  [<ffffffffa02023a4>] call_reserve+0x34/0x60 [sunrpc]
>>  [<ffffffffa020e280>] __rpc_execute+0x90/0x400 [sunrpc]
>>  [<ffffffffa020e61a>] rpc_async_schedule+0x2a/0x40 [sunrpc]
>>  [<ffffffff81073589>] process_one_work+0x139/0x500
>>  [<ffffffff81070e70>] ? alloc_worker+0x70/0x70
>>  [<ffffffffa020e5f0>] ? __rpc_execute+0x400/0x400 [sunrpc]
>>  [<ffffffff81073d1e>] worker_thread+0x15e/0x460
>>  [<ffffffff8145c839>] ? preempt_schedule+0x49/0x70
>>  [<ffffffff81073bc0>] ? rescuer_thread+0x230/0x230
>>  [<ffffffff81079603>] kthread+0x93/0xa0
>>  [<ffffffff81465d04>] kernel_thread_helper+0x4/0x10
>>  [<ffffffff81079570>] ? kthread_freezable_should_stop+0x70/0x70
>>  [<ffffffff81465d00>] ? gs_change+0x13/0x13
>>
>> Signed-off-by: Bryan Schumaker <bjschuma@netapp.com>
>> ---
>>  net/sunrpc/xprtsock.c | 1 +
>>  1 file changed, 1 insertion(+)
>>
>> diff --git a/net/sunrpc/xprtsock.c b/net/sunrpc/xprtsock.c
>> index 86b7777..aaaadfb 100644
>> --- a/net/sunrpc/xprtsock.c
>> +++ b/net/sunrpc/xprtsock.c
>> @@ -2521,6 +2521,7 @@ static struct rpc_xprt_ops xs_tcp_ops = {
>>  static struct rpc_xprt_ops bc_tcp_ops = {
>>  	.reserve_xprt		= xprt_reserve_xprt,
>>  	.release_xprt		= xprt_release_xprt,
>> +	.alloc_slot		= xprt_alloc_slot,
>>  	.rpcbind		= xs_local_rpcbind,
>>  	.buf_alloc		= bc_malloc,
>>  	.buf_free		= bc_free,
>> -- 
>> 1.7.12.1
>>
>> --
>> To unsubscribe from this list: send the line "unsubscribe linux-nfs" in
>> the body of a message to majordomo@vger.kernel.org
>> More majordomo info at  http://vger.kernel.org/majordomo-info.html

--
To unsubscribe from this list: send the line "unsubscribe linux-nfs" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Trond Myklebust Sept. 24, 2012, 7:31 p.m. UTC | #3
On Mon, 2012-09-24 at 13:52 -0400, Bryan Schumaker wrote:
> On 09/24/2012 01:42 PM, J. Bruce Fields wrote:

> > On Mon, Sep 24, 2012 at 01:39:01PM -0400, bjschuma@netapp.com wrote:

> >> From: Bryan Schumaker <bjschuma@netapp.com>

> >>

> >> f39c1bfb5a03e2d255451bff05be0d7255298fa4 (SUNRPC: Fix a UDP transport

> >> regression) introduced the "alloc_slot" function for xprt operations,

> >> but never created one for the backchannel operations.  This patch fixes

> >> a null pointer dereference when mounting NFS over v4.1.

> > 

> > Thanks, I just rebased some of my work to 3.6 and ran across that!  It

> > crashes the 4.1 server very quickly....

> 

> That sounds like my story.  It got my peer-to-peer server right away, too.

> 

> - Bryan

> 

> > 

> > --b.

> > 

> >>

> >> Call Trace:

> >>  [<ffffffffa0207957>] ? xprt_reserve+0x47/0x50 [sunrpc]

> >>  [<ffffffffa02023a4>] call_reserve+0x34/0x60 [sunrpc]

> >>  [<ffffffffa020e280>] __rpc_execute+0x90/0x400 [sunrpc]

> >>  [<ffffffffa020e61a>] rpc_async_schedule+0x2a/0x40 [sunrpc]

> >>  [<ffffffff81073589>] process_one_work+0x139/0x500

> >>  [<ffffffff81070e70>] ? alloc_worker+0x70/0x70

> >>  [<ffffffffa020e5f0>] ? __rpc_execute+0x400/0x400 [sunrpc]

> >>  [<ffffffff81073d1e>] worker_thread+0x15e/0x460

> >>  [<ffffffff8145c839>] ? preempt_schedule+0x49/0x70

> >>  [<ffffffff81073bc0>] ? rescuer_thread+0x230/0x230

> >>  [<ffffffff81079603>] kthread+0x93/0xa0

> >>  [<ffffffff81465d04>] kernel_thread_helper+0x4/0x10

> >>  [<ffffffff81079570>] ? kthread_freezable_should_stop+0x70/0x70

> >>  [<ffffffff81465d00>] ? gs_change+0x13/0x13

> >>

> >> Signed-off-by: Bryan Schumaker <bjschuma@netapp.com>

> >> ---

> >>  net/sunrpc/xprtsock.c | 1 +

> >>  1 file changed, 1 insertion(+)

> >>

> >> diff --git a/net/sunrpc/xprtsock.c b/net/sunrpc/xprtsock.c

> >> index 86b7777..aaaadfb 100644

> >> --- a/net/sunrpc/xprtsock.c

> >> +++ b/net/sunrpc/xprtsock.c

> >> @@ -2521,6 +2521,7 @@ static struct rpc_xprt_ops xs_tcp_ops = {

> >>  static struct rpc_xprt_ops bc_tcp_ops = {

> >>  	.reserve_xprt		= xprt_reserve_xprt,

> >>  	.release_xprt		= xprt_release_xprt,

> >> +	.alloc_slot		= xprt_alloc_slot,

> >>  	.rpcbind		= xs_local_rpcbind,

> >>  	.buf_alloc		= bc_malloc,

> >>  	.buf_free		= bc_free,

> >> -- 

> >> 1.7.12.1

> >>

> >> --

> >> To unsubscribe from this list: send the line "unsubscribe linux-nfs" in

> >> the body of a message to majordomo@vger.kernel.org

> >> More majordomo info at  http://vger.kernel.org/majordomo-info.html

> 


Argh... Sorry, that was entirely my fault. I traced the client side
backchannel code, and found it was allocating slots using its own
mechanism, then thought that applied to bc_tcp_ops.

I find the NFSv4.1 backchannel code to be even more confusing than
lockd.

...and BTW the .rpcbind hack above is a prime example. Bruce, why do you
need that? The server back channel sets xprt_set_bound() in
xs_setup_bc_tcp() and should never clear it.

-- 
Trond Myklebust
Linux NFS client maintainer

NetApp
Trond.Myklebust@netapp.com
www.netapp.com
J. Bruce Fields Oct. 19, 2012, 7:39 p.m. UTC | #4
Did this not get sent to stable?  (Now upstream as
84e28a307e376f271505af65a7b7e212dd6f61f4.)

--b.

On Mon, Sep 24, 2012 at 01:39:01PM -0400, bjschuma@netapp.com wrote:
> From: Bryan Schumaker <bjschuma@netapp.com>
> 
> f39c1bfb5a03e2d255451bff05be0d7255298fa4 (SUNRPC: Fix a UDP transport
> regression) introduced the "alloc_slot" function for xprt operations,
> but never created one for the backchannel operations.  This patch fixes
> a null pointer dereference when mounting NFS over v4.1.
> 
> Call Trace:
>  [<ffffffffa0207957>] ? xprt_reserve+0x47/0x50 [sunrpc]
>  [<ffffffffa02023a4>] call_reserve+0x34/0x60 [sunrpc]
>  [<ffffffffa020e280>] __rpc_execute+0x90/0x400 [sunrpc]
>  [<ffffffffa020e61a>] rpc_async_schedule+0x2a/0x40 [sunrpc]
>  [<ffffffff81073589>] process_one_work+0x139/0x500
>  [<ffffffff81070e70>] ? alloc_worker+0x70/0x70
>  [<ffffffffa020e5f0>] ? __rpc_execute+0x400/0x400 [sunrpc]
>  [<ffffffff81073d1e>] worker_thread+0x15e/0x460
>  [<ffffffff8145c839>] ? preempt_schedule+0x49/0x70
>  [<ffffffff81073bc0>] ? rescuer_thread+0x230/0x230
>  [<ffffffff81079603>] kthread+0x93/0xa0
>  [<ffffffff81465d04>] kernel_thread_helper+0x4/0x10
>  [<ffffffff81079570>] ? kthread_freezable_should_stop+0x70/0x70
>  [<ffffffff81465d00>] ? gs_change+0x13/0x13
> 
> Signed-off-by: Bryan Schumaker <bjschuma@netapp.com>
> ---
>  net/sunrpc/xprtsock.c | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/net/sunrpc/xprtsock.c b/net/sunrpc/xprtsock.c
> index 86b7777..aaaadfb 100644
> --- a/net/sunrpc/xprtsock.c
> +++ b/net/sunrpc/xprtsock.c
> @@ -2521,6 +2521,7 @@ static struct rpc_xprt_ops xs_tcp_ops = {
>  static struct rpc_xprt_ops bc_tcp_ops = {
>  	.reserve_xprt		= xprt_reserve_xprt,
>  	.release_xprt		= xprt_release_xprt,
> +	.alloc_slot		= xprt_alloc_slot,
>  	.rpcbind		= xs_local_rpcbind,
>  	.buf_alloc		= bc_malloc,
>  	.buf_free		= bc_free,
> -- 
> 1.7.12.1
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-nfs" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
--
To unsubscribe from this list: send the line "unsubscribe linux-nfs" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Trond Myklebust Oct. 19, 2012, 8:23 p.m. UTC | #5
Et tu Bruce?

Not yet, because if you check, neither has commit f39c1bfb5a (SUNRPC: Fix a UDP transport regression). I figured it is better to send them both together as a single patch.

Trond

> -----Original Message-----
> From: J. Bruce Fields [mailto:bfields@fieldses.org]
> Sent: Friday, October 19, 2012 3:40 PM
> To: Schumaker, Bryan
> Cc: Myklebust, Trond; linux-nfs@vger.kernel.org; stable@vger.kernel.org
> Subject: Re: [PATCH] SUNRPC: Set alloc_slot for backchannel tcp ops
> 
> Did this not get sent to stable?  (Now upstream as
> 84e28a307e376f271505af65a7b7e212dd6f61f4.)
> 
> --b.
> 
> On Mon, Sep 24, 2012 at 01:39:01PM -0400, bjschuma@netapp.com wrote:
> > From: Bryan Schumaker <bjschuma@netapp.com>
> >
> > f39c1bfb5a03e2d255451bff05be0d7255298fa4 (SUNRPC: Fix a UDP
> transport
> > regression) introduced the "alloc_slot" function for xprt operations,
> > but never created one for the backchannel operations.  This patch
> > fixes a null pointer dereference when mounting NFS over v4.1.
> >
> > Call Trace:
> >  [<ffffffffa0207957>] ? xprt_reserve+0x47/0x50 [sunrpc]
> > [<ffffffffa02023a4>] call_reserve+0x34/0x60 [sunrpc]
> > [<ffffffffa020e280>] __rpc_execute+0x90/0x400 [sunrpc]
> > [<ffffffffa020e61a>] rpc_async_schedule+0x2a/0x40 [sunrpc]
> > [<ffffffff81073589>] process_one_work+0x139/0x500
> > [<ffffffff81070e70>] ? alloc_worker+0x70/0x70  [<ffffffffa020e5f0>] ?
> > __rpc_execute+0x400/0x400 [sunrpc]  [<ffffffff81073d1e>]
> > worker_thread+0x15e/0x460  [<ffffffff8145c839>] ?
> > preempt_schedule+0x49/0x70  [<ffffffff81073bc0>] ?
> > rescuer_thread+0x230/0x230  [<ffffffff81079603>] kthread+0x93/0xa0
> > [<ffffffff81465d04>] kernel_thread_helper+0x4/0x10
> > [<ffffffff81079570>] ? kthread_freezable_should_stop+0x70/0x70
> >  [<ffffffff81465d00>] ? gs_change+0x13/0x13
> >
> > Signed-off-by: Bryan Schumaker <bjschuma@netapp.com>
> > ---
> >  net/sunrpc/xprtsock.c | 1 +
> >  1 file changed, 1 insertion(+)
> >
> > diff --git a/net/sunrpc/xprtsock.c b/net/sunrpc/xprtsock.c index
> > 86b7777..aaaadfb 100644
> > --- a/net/sunrpc/xprtsock.c
> > +++ b/net/sunrpc/xprtsock.c
> > @@ -2521,6 +2521,7 @@ static struct rpc_xprt_ops xs_tcp_ops = {
> > static struct rpc_xprt_ops bc_tcp_ops = {
> >  	.reserve_xprt		= xprt_reserve_xprt,
> >  	.release_xprt		= xprt_release_xprt,
> > +	.alloc_slot		= xprt_alloc_slot,
> >  	.rpcbind		= xs_local_rpcbind,
> >  	.buf_alloc		= bc_malloc,
> >  	.buf_free		= bc_free,
> > --
> > 1.7.12.1
> >
> > --
> > To unsubscribe from this list: send the line "unsubscribe linux-nfs"
> > in the body of a message to majordomo@vger.kernel.org More
> majordomo
> > info at  http://vger.kernel.org/majordomo-info.html
--
To unsubscribe from this list: send the line "unsubscribe linux-nfs" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
J. Bruce Fields Oct. 19, 2012, 9:01 p.m. UTC | #6
On Fri, Oct 19, 2012 at 08:23:05PM +0000, Myklebust, Trond wrote:
> 
> Et tu Bruce?
> 
> Not yet, because if you check, neither has commit f39c1bfb5a (SUNRPC: Fix a UDP transport regression). I figured it is better to send them both together as a single patch.

I'm confused, I thought I did check.  Looking again...  It's in 3.6:

	$ git log v3.6..f39c1bfb5a
	$ 

and cc'd to stable:

	$ git show f39c1bfb5a|grep stable
	    Cc: stable@vger.kernel.org [>= 3.1]

so presumably it's already on its way to older stable branches too.

--b.

> 
> Trond
> 
> > -----Original Message-----
> > From: J. Bruce Fields [mailto:bfields@fieldses.org]
> > Sent: Friday, October 19, 2012 3:40 PM
> > To: Schumaker, Bryan
> > Cc: Myklebust, Trond; linux-nfs@vger.kernel.org; stable@vger.kernel.org
> > Subject: Re: [PATCH] SUNRPC: Set alloc_slot for backchannel tcp ops
> > 
> > Did this not get sent to stable?  (Now upstream as
> > 84e28a307e376f271505af65a7b7e212dd6f61f4.)
> > 
> > --b.
> > 
> > On Mon, Sep 24, 2012 at 01:39:01PM -0400, bjschuma@netapp.com wrote:
> > > From: Bryan Schumaker <bjschuma@netapp.com>
> > >
> > > f39c1bfb5a03e2d255451bff05be0d7255298fa4 (SUNRPC: Fix a UDP
> > transport
> > > regression) introduced the "alloc_slot" function for xprt operations,
> > > but never created one for the backchannel operations.  This patch
> > > fixes a null pointer dereference when mounting NFS over v4.1.
> > >
> > > Call Trace:
> > >  [<ffffffffa0207957>] ? xprt_reserve+0x47/0x50 [sunrpc]
> > > [<ffffffffa02023a4>] call_reserve+0x34/0x60 [sunrpc]
> > > [<ffffffffa020e280>] __rpc_execute+0x90/0x400 [sunrpc]
> > > [<ffffffffa020e61a>] rpc_async_schedule+0x2a/0x40 [sunrpc]
> > > [<ffffffff81073589>] process_one_work+0x139/0x500
> > > [<ffffffff81070e70>] ? alloc_worker+0x70/0x70  [<ffffffffa020e5f0>] ?
> > > __rpc_execute+0x400/0x400 [sunrpc]  [<ffffffff81073d1e>]
> > > worker_thread+0x15e/0x460  [<ffffffff8145c839>] ?
> > > preempt_schedule+0x49/0x70  [<ffffffff81073bc0>] ?
> > > rescuer_thread+0x230/0x230  [<ffffffff81079603>] kthread+0x93/0xa0
> > > [<ffffffff81465d04>] kernel_thread_helper+0x4/0x10
> > > [<ffffffff81079570>] ? kthread_freezable_should_stop+0x70/0x70
> > >  [<ffffffff81465d00>] ? gs_change+0x13/0x13
> > >
> > > Signed-off-by: Bryan Schumaker <bjschuma@netapp.com>
> > > ---
> > >  net/sunrpc/xprtsock.c | 1 +
> > >  1 file changed, 1 insertion(+)
> > >
> > > diff --git a/net/sunrpc/xprtsock.c b/net/sunrpc/xprtsock.c index
> > > 86b7777..aaaadfb 100644
> > > --- a/net/sunrpc/xprtsock.c
> > > +++ b/net/sunrpc/xprtsock.c
> > > @@ -2521,6 +2521,7 @@ static struct rpc_xprt_ops xs_tcp_ops = {
> > > static struct rpc_xprt_ops bc_tcp_ops = {
> > >  	.reserve_xprt		= xprt_reserve_xprt,
> > >  	.release_xprt		= xprt_release_xprt,
> > > +	.alloc_slot		= xprt_alloc_slot,
> > >  	.rpcbind		= xs_local_rpcbind,
> > >  	.buf_alloc		= bc_malloc,
> > >  	.buf_free		= bc_free,
> > > --
> > > 1.7.12.1
> > >
> > > --
> > > To unsubscribe from this list: send the line "unsubscribe linux-nfs"
> > > in the body of a message to majordomo@vger.kernel.org More
> > majordomo
> > > info at  http://vger.kernel.org/majordomo-info.html
--
To unsubscribe from this list: send the line "unsubscribe linux-nfs" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Trond Myklebust Oct. 19, 2012, 9:05 p.m. UTC | #7
> -----Original Message-----
> From: J. Bruce Fields [mailto:bfields@fieldses.org]
> Sent: Friday, October 19, 2012 5:01 PM
> To: Myklebust, Trond
> Cc: Schumaker, Bryan; linux-nfs@vger.kernel.org; stable@vger.kernel.org
> Subject: Re: [PATCH] SUNRPC: Set alloc_slot for backchannel tcp ops
> 
> On Fri, Oct 19, 2012 at 08:23:05PM +0000, Myklebust, Trond wrote:
> >
> > Et tu Bruce?
> >
> > Not yet, because if you check, neither has commit f39c1bfb5a (SUNRPC: Fix
> a UDP transport regression). I figured it is better to send them both together
> as a single patch.
> 
> I'm confused, I thought I did check.  Looking again...  It's in 3.6:
> 
> 	$ git log v3.6..f39c1bfb5a
> 	$

Yes. 3.6 still has the original patch+problem.

> and cc'd to stable:
> 
> 	$ git show f39c1bfb5a|grep stable
> 	    Cc: stable@vger.kernel.org [>= 3.1]
> 
> so presumably it's already on its way to older stable branches too.

No. See attachment... 

 
> >
> > Trond
> >
> > > -----Original Message-----
> > > From: J. Bruce Fields [mailto:bfields@fieldses.org]
> > > Sent: Friday, October 19, 2012 3:40 PM
> > > To: Schumaker, Bryan
> > > Cc: Myklebust, Trond; linux-nfs@vger.kernel.org;
> > > stable@vger.kernel.org
> > > Subject: Re: [PATCH] SUNRPC: Set alloc_slot for backchannel tcp ops
> > >
> > > Did this not get sent to stable?  (Now upstream as
> > > 84e28a307e376f271505af65a7b7e212dd6f61f4.)
> > >
> > > --b.
> > >
> > > On Mon, Sep 24, 2012 at 01:39:01PM -0400, bjschuma@netapp.com
> wrote:
> > > > From: Bryan Schumaker <bjschuma@netapp.com>
> > > >
> > > > f39c1bfb5a03e2d255451bff05be0d7255298fa4 (SUNRPC: Fix a UDP
> > > transport
> > > > regression) introduced the "alloc_slot" function for xprt
> > > > operations, but never created one for the backchannel operations.
> > > > This patch fixes a null pointer dereference when mounting NFS over
> v4.1.
> > > >
> > > > Call Trace:
> > > >  [<ffffffffa0207957>] ? xprt_reserve+0x47/0x50 [sunrpc]
> > > > [<ffffffffa02023a4>] call_reserve+0x34/0x60 [sunrpc]
> > > > [<ffffffffa020e280>] __rpc_execute+0x90/0x400 [sunrpc]
> > > > [<ffffffffa020e61a>] rpc_async_schedule+0x2a/0x40 [sunrpc]
> > > > [<ffffffff81073589>] process_one_work+0x139/0x500
> > > > [<ffffffff81070e70>] ? alloc_worker+0x70/0x70  [<ffffffffa020e5f0>] ?
> > > > __rpc_execute+0x400/0x400 [sunrpc]  [<ffffffff81073d1e>]
> > > > worker_thread+0x15e/0x460  [<ffffffff8145c839>] ?
> > > > preempt_schedule+0x49/0x70  [<ffffffff81073bc0>] ?
> > > > rescuer_thread+0x230/0x230  [<ffffffff81079603>] kthread+0x93/0xa0
> > > > [<ffffffff81465d04>] kernel_thread_helper+0x4/0x10
> > > > [<ffffffff81079570>] ? kthread_freezable_should_stop+0x70/0x70
> > > >  [<ffffffff81465d00>] ? gs_change+0x13/0x13
> > > >
> > > > Signed-off-by: Bryan Schumaker <bjschuma@netapp.com>
> > > > ---
> > > >  net/sunrpc/xprtsock.c | 1 +
> > > >  1 file changed, 1 insertion(+)
> > > >
> > > > diff --git a/net/sunrpc/xprtsock.c b/net/sunrpc/xprtsock.c index
> > > > 86b7777..aaaadfb 100644
> > > > --- a/net/sunrpc/xprtsock.c
> > > > +++ b/net/sunrpc/xprtsock.c
> > > > @@ -2521,6 +2521,7 @@ static struct rpc_xprt_ops xs_tcp_ops = {
> > > > static struct rpc_xprt_ops bc_tcp_ops = {
> > > >  	.reserve_xprt		= xprt_reserve_xprt,
> > > >  	.release_xprt		= xprt_release_xprt,
> > > > +	.alloc_slot		= xprt_alloc_slot,
> > > >  	.rpcbind		= xs_local_rpcbind,
> > > >  	.buf_alloc		= bc_malloc,
> > > >  	.buf_free		= bc_free,
> > > > --
> > > > 1.7.12.1
> > > >
> > > > --
> > > > To unsubscribe from this list: send the line "unsubscribe linux-nfs"
> > > > in the body of a message to majordomo@vger.kernel.org More
> > > majordomo
> > > > info at  http://vger.kernel.org/majordomo-info.html
On Fri, 2012-09-28 at 13:14 -0700, Greg Kroah-Hartman wrote:
> 3.4-stable review patch.  If anyone has any objections, please let me know.

>

> ------------------

>

> From: Trond Myklebust <Trond.Myklebust@netapp.com>

>

> commit f39c1bfb5a03e2d255451bff05be0d7255298fa4 upstream.

>

> Commit 43cedbf0e8dfb9c5610eb7985d5f21263e313802 (SUNRPC: Ensure that

> we grab the XPRT_LOCK before calling xprt_alloc_slot) is causing

> hangs in the case of NFS over UDP mounts.


Ditto with this. Please hold this patch until the 1 line fix mentioned
previously has been merged upstream (at which point I will resend the
patch through stable@vger.kernel.org).

Thanks!
  Trond

--
Trond Myklebust
Linux NFS client maintainer

NetApp
Trond.Myklebust@netapp.com
www.netapp.com
J. Bruce Fields Oct. 19, 2012, 9:11 p.m. UTC | #8
On Fri, Oct 19, 2012 at 09:05:08PM +0000, Myklebust, Trond wrote:
> > -----Original Message-----
> > From: J. Bruce Fields [mailto:bfields@fieldses.org]
> > Sent: Friday, October 19, 2012 5:01 PM
> > To: Myklebust, Trond
> > Cc: Schumaker, Bryan; linux-nfs@vger.kernel.org; stable@vger.kernel.org
> > Subject: Re: [PATCH] SUNRPC: Set alloc_slot for backchannel tcp ops
> > 
> > On Fri, Oct 19, 2012 at 08:23:05PM +0000, Myklebust, Trond wrote:
> > >
> > > Et tu Bruce?
> > >
> > > Not yet, because if you check, neither has commit f39c1bfb5a (SUNRPC: Fix
> > a UDP transport regression). I figured it is better to send them both together
> > as a single patch.
> > 
> > I'm confused, I thought I did check.  Looking again...  It's in 3.6:
> > 
> > 	$ git log v3.6..f39c1bfb5a
> > 	$
> 
> Yes. 3.6 still has the original patch+problem.
> 
> > and cc'd to stable:
> > 
> > 	$ git show f39c1bfb5a|grep stable
> > 	    Cc: stable@vger.kernel.org [>= 3.1]
> > 
> > so presumably it's already on its way to older stable branches too.
> 
> No. See attachment... 

OK.  As long as it makes it to 3.6.x one way or another, I'm happy....

--b.
--
To unsubscribe from this list: send the line "unsubscribe linux-nfs" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Ben Hutchings Oct. 27, 2012, 11:25 p.m. UTC | #9
On Fri, 2012-10-19 at 15:39 -0400, J. Bruce Fields wrote:
> Did this not get sent to stable?  (Now upstream as
> 84e28a307e376f271505af65a7b7e212dd6f61f4.)
[...]

I've added this to the queue for 3.2.

Ben.
diff mbox

Patch

diff --git a/net/sunrpc/xprtsock.c b/net/sunrpc/xprtsock.c
index 86b7777..aaaadfb 100644
--- a/net/sunrpc/xprtsock.c
+++ b/net/sunrpc/xprtsock.c
@@ -2521,6 +2521,7 @@  static struct rpc_xprt_ops xs_tcp_ops = {
 static struct rpc_xprt_ops bc_tcp_ops = {
 	.reserve_xprt		= xprt_reserve_xprt,
 	.release_xprt		= xprt_release_xprt,
+	.alloc_slot		= xprt_alloc_slot,
 	.rpcbind		= xs_local_rpcbind,
 	.buf_alloc		= bc_malloc,
 	.buf_free		= bc_free,