diff mbox

Issues with a rather unusual configured NFS server

Message ID 20130827180655.GE14809@fieldses.org
State Not Applicable, archived
Headers show

Commit Message

J. Bruce Fields Aug. 27, 2013, 6:06 p.m. UTC
On Tue, Aug 13, 2013 at 05:53:14PM -0400, bfields wrote:
> On Mon, Aug 12, 2013 at 04:36:40PM +0200, Jan Kara wrote:
> > On Sun 11-08-13 11:48:49, Toralf Förster wrote:
> > > so that the server either crashes (if it is a user mode linux image) or at least its reboot functionality got broken
> > > - if the NFS server is hammered with scary NFS calls using a fuzzy tool running at a remote NFS client under a non-privileged user id.
> > > 
> > > It can re reproduced, if
> > > 	- the NFS share is an EXT3 or EXT4 directory
> > > 	- and it is created at file located at tempfs and mounted via loop device
> > > 	- and the NFS server is forced to umount the NFS share
> > > 	- and the server forced to restart the NSF service afterwards
> > > 	- and trinity is used
> > > 
> > > I could find a scenario for an automated bisect. 2 times it brought this commit 
> > > commit 68a3396178e6688ad7367202cdf0af8ed03c8727
> > > Author: J. Bruce Fields <bfields@redhat.com>
> > > Date:   Thu Mar 21 11:21:50 2013 -0400
> > > 
> > >     nfsd4: shut down more of delegation earlier
> 
> Thanks for the report.  I think I see the problem--after this commit
> nfs4_set_delegation() failures result in nfs4_put_delegation being
> called, but nfs4_put_delegation doesn't free the nfs4_file that has
> already been set by alloc_init_deleg().
> 
> Let me think about how to fix that....

Sorry for the slow response--can you check whether this fixes the
problem?

--b.

commit 624a0ee0375940ce4aa36330b0b5a70af6d2b6f5
Author: J. Bruce Fields <bfields@redhat.com>
Date:   Thu Aug 15 16:55:26 2013 -0400

    nfsd4: fix leak of inode reference on delegation failure
    
    This fixes a regression from 68a3396178e6688ad7367202cdf0af8ed03c8727
    "nfsd4: shut down more of delegation earlier".
    
    After that commit, nfs4_set_delegation() failures result in
    nfs4_put_delegation being called, but nfs4_put_delegation doesn't free
    the nfs4_file that has already been set by alloc_init_deleg().
    
    This can result in an oops on later unmounting the exported filesystem.
    
    Note also delaying the fi_had_conflict check we're able to return a
    better error (hence give 4.1 clients a better idea why the delegation
    failed; though note CONFLICT isn't an exact match here, as that's
    supposed to indicate a current conflict, but all we know here is that
    there was one recently).
    
    Reported-by: Toralf Förster <toralf.foerster@gmx.de>
    Signed-off-by: J. Bruce Fields <bfields@redhat.com>

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

Comments

Toralf Förster Aug. 28, 2013, 5:21 p.m. UTC | #1
On 08/27/2013 08:06 PM, J. Bruce Fields wrote:
> On Tue, Aug 13, 2013 at 05:53:14PM -0400, bfields wrote:
>> On Mon, Aug 12, 2013 at 04:36:40PM +0200, Jan Kara wrote:
>>> On Sun 11-08-13 11:48:49, Toralf Förster wrote:
>>>> so that the server either crashes (if it is a user mode linux image) or at least its reboot functionality got broken
>>>> - if the NFS server is hammered with scary NFS calls using a fuzzy tool running at a remote NFS client under a non-privileged user id.
>>>>
>>>> It can re reproduced, if
>>>> 	- the NFS share is an EXT3 or EXT4 directory
>>>> 	- and it is created at file located at tempfs and mounted via loop device
>>>> 	- and the NFS server is forced to umount the NFS share
>>>> 	- and the server forced to restart the NSF service afterwards
>>>> 	- and trinity is used
>>>>
>>>> I could find a scenario for an automated bisect. 2 times it brought this commit 
>>>> commit 68a3396178e6688ad7367202cdf0af8ed03c8727
>>>> Author: J. Bruce Fields <bfields@redhat.com>
>>>> Date:   Thu Mar 21 11:21:50 2013 -0400
>>>>
>>>>     nfsd4: shut down more of delegation earlier
>>
>> Thanks for the report.  I think I see the problem--after this commit
>> nfs4_set_delegation() failures result in nfs4_put_delegation being
>> called, but nfs4_put_delegation doesn't free the nfs4_file that has
>> already been set by alloc_init_deleg().
>>
>> Let me think about how to fix that....
> 
> Sorry for the slow response--can you check whether this fixes the
> problem?
> 
Yes.

With the attached patch the problem can't be reproduced any longer with
the prepared test case and current git kernels.

> --b.
> 
> commit 624a0ee0375940ce4aa36330b0b5a70af6d2b6f5
> Author: J. Bruce Fields <bfields@redhat.com>
> Date:   Thu Aug 15 16:55:26 2013 -0400
> 
>     nfsd4: fix leak of inode reference on delegation failure
>     
>     This fixes a regression from 68a3396178e6688ad7367202cdf0af8ed03c8727
>     "nfsd4: shut down more of delegation earlier".
>     
>     After that commit, nfs4_set_delegation() failures result in
>     nfs4_put_delegation being called, but nfs4_put_delegation doesn't free
>     the nfs4_file that has already been set by alloc_init_deleg().
>     
>     This can result in an oops on later unmounting the exported filesystem.
>     
>     Note also delaying the fi_had_conflict check we're able to return a
>     better error (hence give 4.1 clients a better idea why the delegation
>     failed; though note CONFLICT isn't an exact match here, as that's
>     supposed to indicate a current conflict, but all we know here is that
>     there was one recently).
>     
>     Reported-by: Toralf Förster <toralf.foerster@gmx.de>
>     Signed-off-by: J. Bruce Fields <bfields@redhat.com>
> 
> diff --git a/fs/nfsd/nfs4state.c b/fs/nfsd/nfs4state.c
> index eb9cf81..0874998 100644
> --- a/fs/nfsd/nfs4state.c
> +++ b/fs/nfsd/nfs4state.c
> @@ -368,11 +368,8 @@ static struct nfs4_delegation *
>  alloc_init_deleg(struct nfs4_client *clp, struct nfs4_ol_stateid *stp, struct svc_fh *current_fh)
>  {
>  	struct nfs4_delegation *dp;
> -	struct nfs4_file *fp = stp->st_file;
>  
>  	dprintk("NFSD alloc_init_deleg\n");
> -	if (fp->fi_had_conflict)
> -		return NULL;
>  	if (num_delegations > max_delegations)
>  		return NULL;
>  	dp = delegstateid(nfs4_alloc_stid(clp, deleg_slab));
> @@ -389,8 +386,7 @@ alloc_init_deleg(struct nfs4_client *clp, struct nfs4_ol_stateid *stp, struct sv
>  	INIT_LIST_HEAD(&dp->dl_perfile);
>  	INIT_LIST_HEAD(&dp->dl_perclnt);
>  	INIT_LIST_HEAD(&dp->dl_recall_lru);
> -	get_nfs4_file(fp);
> -	dp->dl_file = fp;
> +	dp->dl_file = NULL;
>  	dp->dl_type = NFS4_OPEN_DELEGATE_READ;
>  	fh_copy_shallow(&dp->dl_fh, &current_fh->fh_handle);
>  	dp->dl_time = 0;
> @@ -3044,22 +3040,35 @@ static int nfs4_setlease(struct nfs4_delegation *dp)
>  	return 0;
>  }
>  
> -static int nfs4_set_delegation(struct nfs4_delegation *dp)
> +static int nfs4_set_delegation(struct nfs4_delegation *dp, struct nfs4_file *fp)
>  {
> -	struct nfs4_file *fp = dp->dl_file;
> +	int status;
>  
> -	if (!fp->fi_lease)
> -		return nfs4_setlease(dp);
> +	if (fp->fi_had_conflict)
> +		return -EAGAIN;
> +	get_nfs4_file(fp);
> +	dp->dl_file = fp;
> +	if (!fp->fi_lease) {
> +		status = nfs4_setlease(dp);
> +		if (status)
> +			goto out_free;
> +		return 0;
> +	}
>  	spin_lock(&recall_lock);
>  	if (fp->fi_had_conflict) {
>  		spin_unlock(&recall_lock);
> -		return -EAGAIN;
> +		status = -EAGAIN;
> +		goto out_free;
>  	}
>  	atomic_inc(&fp->fi_delegees);
>  	list_add(&dp->dl_perfile, &fp->fi_delegations);
>  	spin_unlock(&recall_lock);
>  	list_add(&dp->dl_perclnt, &dp->dl_stid.sc_client->cl_delegations);
>  	return 0;
> +out_free:
> +	put_nfs4_file(fp);
> +	dp->dl_file = fp;
> +	return status;
>  }
>  
>  static void nfsd4_open_deleg_none_ext(struct nfsd4_open *open, int status)
> @@ -3134,7 +3143,7 @@ nfs4_open_delegation(struct net *net, struct svc_fh *fh,
>  	dp = alloc_init_deleg(oo->oo_owner.so_client, stp, fh);
>  	if (dp == NULL)
>  		goto out_no_deleg;
> -	status = nfs4_set_delegation(dp);
> +	status = nfs4_set_delegation(dp, stp->st_file);
>  	if (status)
>  		goto out_free;
>  
>
Richard Weinberger Aug. 29, 2013, 9:57 a.m. UTC | #2
On Wed, Aug 28, 2013 at 7:21 PM, Toralf Förster <toralf.foerster@gmx.de> wrote:
> On 08/27/2013 08:06 PM, J. Bruce Fields wrote:
>> On Tue, Aug 13, 2013 at 05:53:14PM -0400, bfields wrote:
>>> On Mon, Aug 12, 2013 at 04:36:40PM +0200, Jan Kara wrote:
>>>> On Sun 11-08-13 11:48:49, Toralf Förster wrote:
>>>>> so that the server either crashes (if it is a user mode linux image) or at least its reboot functionality got broken
>>>>> - if the NFS server is hammered with scary NFS calls using a fuzzy tool running at a remote NFS client under a non-privileged user id.
>>>>>
>>>>> It can re reproduced, if
>>>>>    - the NFS share is an EXT3 or EXT4 directory
>>>>>    - and it is created at file located at tempfs and mounted via loop device
>>>>>    - and the NFS server is forced to umount the NFS share
>>>>>    - and the server forced to restart the NSF service afterwards
>>>>>    - and trinity is used
>>>>>
>>>>> I could find a scenario for an automated bisect. 2 times it brought this commit
>>>>> commit 68a3396178e6688ad7367202cdf0af8ed03c8727
>>>>> Author: J. Bruce Fields <bfields@redhat.com>
>>>>> Date:   Thu Mar 21 11:21:50 2013 -0400
>>>>>
>>>>>     nfsd4: shut down more of delegation earlier
>>>
>>> Thanks for the report.  I think I see the problem--after this commit
>>> nfs4_set_delegation() failures result in nfs4_put_delegation being
>>> called, but nfs4_put_delegation doesn't free the nfs4_file that has
>>> already been set by alloc_init_deleg().
>>>
>>> Let me think about how to fix that....
>>
>> Sorry for the slow response--can you check whether this fixes the
>> problem?
>>
> Yes.
>
> With the attached patch the problem can't be reproduced any longer with
> the prepared test case and current git kernels.

BTW: Is nobody else fuzz testing NFS?
Or are these bugs just more likely to hit on UML?
This is not the first NFS issue found by Toralf using UML and Trinity.
J. Bruce Fields Aug. 29, 2013, 1:30 p.m. UTC | #3
On Thu, Aug 29, 2013 at 11:57:45AM +0200, richard -rw- weinberger wrote:
> On Wed, Aug 28, 2013 at 7:21 PM, Toralf Förster <toralf.foerster@gmx.de> wrote:
> > On 08/27/2013 08:06 PM, J. Bruce Fields wrote:
> >> On Tue, Aug 13, 2013 at 05:53:14PM -0400, bfields wrote:
> >>> On Mon, Aug 12, 2013 at 04:36:40PM +0200, Jan Kara wrote:
> >>>> On Sun 11-08-13 11:48:49, Toralf Förster wrote:
> >>>>> so that the server either crashes (if it is a user mode linux image) or at least its reboot functionality got broken
> >>>>> - if the NFS server is hammered with scary NFS calls using a fuzzy tool running at a remote NFS client under a non-privileged user id.
> >>>>>
> >>>>> It can re reproduced, if
> >>>>>    - the NFS share is an EXT3 or EXT4 directory
> >>>>>    - and it is created at file located at tempfs and mounted via loop device
> >>>>>    - and the NFS server is forced to umount the NFS share
> >>>>>    - and the server forced to restart the NSF service afterwards
> >>>>>    - and trinity is used
> >>>>>
> >>>>> I could find a scenario for an automated bisect. 2 times it brought this commit
> >>>>> commit 68a3396178e6688ad7367202cdf0af8ed03c8727
> >>>>> Author: J. Bruce Fields <bfields@redhat.com>
> >>>>> Date:   Thu Mar 21 11:21:50 2013 -0400
> >>>>>
> >>>>>     nfsd4: shut down more of delegation earlier
> >>>
> >>> Thanks for the report.  I think I see the problem--after this commit
> >>> nfs4_set_delegation() failures result in nfs4_put_delegation being
> >>> called, but nfs4_put_delegation doesn't free the nfs4_file that has
> >>> already been set by alloc_init_deleg().
> >>>
> >>> Let me think about how to fix that....
> >>
> >> Sorry for the slow response--can you check whether this fixes the
> >> problem?
> >>
> > Yes.
> >
> > With the attached patch the problem can't be reproduced any longer with
> > the prepared test case and current git kernels.
> 
> BTW: Is nobody else fuzz testing NFS?

I don't know.  Toralf's reports are the only ones I recall off the top
of my head, but I may have forgotten others.

> Or are these bugs just more likely to hit on UML?

That's also possible.

> This is not the first NFS issue found by Toralf using UML and Trinity.

Yep.  The testing is definitely appreciated.

--b.
--
To unsubscribe from this list: send the line "unsubscribe linux-ext4" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Toralf Förster Aug. 30, 2013, 2:10 p.m. UTC | #4
On 08/29/2013 03:30 PM, J. Bruce Fields wrote:
> On Thu, Aug 29, 2013 at 11:57:45AM +0200, richard -rw- weinberger wrote:
>> On Wed, Aug 28, 2013 at 7:21 PM, Toralf Förster <toralf.foerster@gmx.de> wrote:
>>> On 08/27/2013 08:06 PM, J. Bruce Fields wrote:
>>>> On Tue, Aug 13, 2013 at 05:53:14PM -0400, bfields wrote:
>>>>> On Mon, Aug 12, 2013 at 04:36:40PM +0200, Jan Kara wrote:
>>>>>> On Sun 11-08-13 11:48:49, Toralf Förster wrote:
>>>>>>> so that the server either crashes (if it is a user mode linux image) or at least its reboot functionality got broken
>>>>>>> - if the NFS server is hammered with scary NFS calls using a fuzzy tool running at a remote NFS client under a non-privileged user id.
>>>>>>>
>>>>>>> It can re reproduced, if
>>>>>>>    - the NFS share is an EXT3 or EXT4 directory
>>>>>>>    - and it is created at file located at tempfs and mounted via loop device
>>>>>>>    - and the NFS server is forced to umount the NFS share
>>>>>>>    - and the server forced to restart the NSF service afterwards
>>>>>>>    - and trinity is used
>>>>>>>
>>>>>>> I could find a scenario for an automated bisect. 2 times it brought this commit
>>>>>>> commit 68a3396178e6688ad7367202cdf0af8ed03c8727
>>>>>>> Author: J. Bruce Fields <bfields@redhat.com>
>>>>>>> Date:   Thu Mar 21 11:21:50 2013 -0400
>>>>>>>
>>>>>>>     nfsd4: shut down more of delegation earlier
>>>>>
>>>>> Thanks for the report.  I think I see the problem--after this commit
>>>>> nfs4_set_delegation() failures result in nfs4_put_delegation being
>>>>> called, but nfs4_put_delegation doesn't free the nfs4_file that has
>>>>> already been set by alloc_init_deleg().
>>>>>
>>>>> Let me think about how to fix that....
>>>>
>>>> Sorry for the slow response--can you check whether this fixes the
>>>> problem?
>>>>
>>> Yes.
>>>
>>> With the attached patch the problem can't be reproduced any longer with
>>> the prepared test case and current git kernels.
>>
>> BTW: Is nobody else fuzz testing NFS?
> 
> I don't know.  Toralf's reports are the only ones I recall off the top
> of my head, but I may have forgotten others.
> 

well, 7255e71 and 3c50ba8 I'd say.

>> Or are these bugs just more likely to hit on UML?

This definitely not. I observed at a real system EXT4 corruptions/
issues but reported them to the EXT4 mailing list.
It just took me a longer time to figure out a reliable configuration
with 2 UML machiens to automatic bisect it.


> That's also possible.
> 
>> This is not the first NFS issue found by Toralf using UML and Trinity.
> 
> Yep.  The testing is definitely appreciated.

Thx - in the mean while although my UML bisect scripts are working fine
and trinity is stable enough even in UML environments to be trust worth.

> 
> --b.
>
J. Bruce Fields Aug. 30, 2013, 2:25 p.m. UTC | #5
On Fri, Aug 30, 2013 at 04:10:42PM +0200, Toralf Förster wrote:
> On 08/29/2013 03:30 PM, J. Bruce Fields wrote:
> > On Thu, Aug 29, 2013 at 11:57:45AM +0200, richard -rw- weinberger wrote:
> >> On Wed, Aug 28, 2013 at 7:21 PM, Toralf Förster <toralf.foerster@gmx.de> wrote:
> >>> On 08/27/2013 08:06 PM, J. Bruce Fields wrote:
> >>>> On Tue, Aug 13, 2013 at 05:53:14PM -0400, bfields wrote:
> >>>>> On Mon, Aug 12, 2013 at 04:36:40PM +0200, Jan Kara wrote:
> >>>>>> On Sun 11-08-13 11:48:49, Toralf Förster wrote:
> >>>>>>> so that the server either crashes (if it is a user mode linux image) or at least its reboot functionality got broken
> >>>>>>> - if the NFS server is hammered with scary NFS calls using a fuzzy tool running at a remote NFS client under a non-privileged user id.
> >>>>>>>
> >>>>>>> It can re reproduced, if
> >>>>>>>    - the NFS share is an EXT3 or EXT4 directory
> >>>>>>>    - and it is created at file located at tempfs and mounted via loop device
> >>>>>>>    - and the NFS server is forced to umount the NFS share
> >>>>>>>    - and the server forced to restart the NSF service afterwards
> >>>>>>>    - and trinity is used
> >>>>>>>
> >>>>>>> I could find a scenario for an automated bisect. 2 times it brought this commit
> >>>>>>> commit 68a3396178e6688ad7367202cdf0af8ed03c8727
> >>>>>>> Author: J. Bruce Fields <bfields@redhat.com>
> >>>>>>> Date:   Thu Mar 21 11:21:50 2013 -0400
> >>>>>>>
> >>>>>>>     nfsd4: shut down more of delegation earlier
> >>>>>
> >>>>> Thanks for the report.  I think I see the problem--after this commit
> >>>>> nfs4_set_delegation() failures result in nfs4_put_delegation being
> >>>>> called, but nfs4_put_delegation doesn't free the nfs4_file that has
> >>>>> already been set by alloc_init_deleg().
> >>>>>
> >>>>> Let me think about how to fix that....
> >>>>
> >>>> Sorry for the slow response--can you check whether this fixes the
> >>>> problem?
> >>>>
> >>> Yes.
> >>>
> >>> With the attached patch the problem can't be reproduced any longer with
> >>> the prepared test case and current git kernels.
> >>
> >> BTW: Is nobody else fuzz testing NFS?
> > 
> > I don't know.  Toralf's reports are the only ones I recall off the top
> > of my head, but I may have forgotten others.
> > 
> 
> well, 7255e71 and 3c50ba8 I'd say.

I don't know any 3c50ba8.  0c7c3e67 "nfsd4: don't close read-write opens
too soon" and 64a817cf "nfsd4: reject "negative" acl lengths" are two
other serious bugs found by your testing.  I don't recall fuzz testing
results from anyone else, but as I say I may have forgotten.

--b.
--
To unsubscribe from this list: send the line "unsubscribe linux-ext4" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Richard Weinberger Aug. 30, 2013, 2:36 p.m. UTC | #6
Am 30.08.2013 16:10, schrieb Toralf Förster:
> On 08/29/2013 03:30 PM, J. Bruce Fields wrote:
>> On Thu, Aug 29, 2013 at 11:57:45AM +0200, richard -rw- weinberger wrote:
>>> On Wed, Aug 28, 2013 at 7:21 PM, Toralf Förster <toralf.foerster@gmx.de> wrote:
>>>> On 08/27/2013 08:06 PM, J. Bruce Fields wrote:
>>>>> On Tue, Aug 13, 2013 at 05:53:14PM -0400, bfields wrote:
>>>>>> On Mon, Aug 12, 2013 at 04:36:40PM +0200, Jan Kara wrote:
>>>>>>> On Sun 11-08-13 11:48:49, Toralf Förster wrote:
>>>>>>>> so that the server either crashes (if it is a user mode linux image) or at least its reboot functionality got broken
>>>>>>>> - if the NFS server is hammered with scary NFS calls using a fuzzy tool running at a remote NFS client under a non-privileged user id.
>>>>>>>>
>>>>>>>> It can re reproduced, if
>>>>>>>>    - the NFS share is an EXT3 or EXT4 directory
>>>>>>>>    - and it is created at file located at tempfs and mounted via loop device
>>>>>>>>    - and the NFS server is forced to umount the NFS share
>>>>>>>>    - and the server forced to restart the NSF service afterwards
>>>>>>>>    - and trinity is used
>>>>>>>>
>>>>>>>> I could find a scenario for an automated bisect. 2 times it brought this commit
>>>>>>>> commit 68a3396178e6688ad7367202cdf0af8ed03c8727
>>>>>>>> Author: J. Bruce Fields <bfields@redhat.com>
>>>>>>>> Date:   Thu Mar 21 11:21:50 2013 -0400
>>>>>>>>
>>>>>>>>     nfsd4: shut down more of delegation earlier
>>>>>>
>>>>>> Thanks for the report.  I think I see the problem--after this commit
>>>>>> nfs4_set_delegation() failures result in nfs4_put_delegation being
>>>>>> called, but nfs4_put_delegation doesn't free the nfs4_file that has
>>>>>> already been set by alloc_init_deleg().
>>>>>>
>>>>>> Let me think about how to fix that....
>>>>>
>>>>> Sorry for the slow response--can you check whether this fixes the
>>>>> problem?
>>>>>
>>>> Yes.
>>>>
>>>> With the attached patch the problem can't be reproduced any longer with
>>>> the prepared test case and current git kernels.
>>>
>>> BTW: Is nobody else fuzz testing NFS?
>>
>> I don't know.  Toralf's reports are the only ones I recall off the top
>> of my head, but I may have forgotten others.
>>
> 
> well, 7255e71 and 3c50ba8 I'd say.
> 
>>> Or are these bugs just more likely to hit on UML?
> 
> This definitely not. I observed at a real system EXT4 corruptions/
> issues but reported them to the EXT4 mailing list.
> It just took me a longer time to figure out a reliable configuration
> with 2 UML machiens to automatic bisect it.
> 
> 
>> That's also possible.
>>
>>> This is not the first NFS issue found by Toralf using UML and Trinity.
>>
>> Yep.  The testing is definitely appreciated.
> 
> Thx - in the mean while although my UML bisect scripts are working fine
> and trinity is stable enough even in UML environments to be trust worth.

That's good to know.
Thanks you and trinity we got rid of some nasty UML bugs.

Thanks,
//richard

--
To unsubscribe from this list: send the line "unsubscribe linux-ext4" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Michael Richardson Aug. 30, 2013, 6:27 p.m. UTC | #7
richard -rw- weinberger <richard.weinberger@gmail.com> wrote:
    >>>> Thanks for the report.  I think I see the problem--after this commit
    >>>> nfs4_set_delegation() failures result in nfs4_put_delegation being
    >>>> called, but nfs4_put_delegation doesn't free the nfs4_file that has
    >>>> already been set by alloc_init_deleg().
    >>>>
    >>>> Let me think about how to fix that....
    >>>
    >>> Sorry for the slow response--can you check whether this fixes the
    >>> problem?
    >>>
    >> Yes.
    >>
    >> With the attached patch the problem can't be reproduced any longer
    >> with the prepared test case and current git kernels.

    > BTW: Is nobody else fuzz testing NFS?  Or are these bugs just more
    > likely to hit on UML?  This is not the first NFS issue found by Toralf
    > using UML and Trinity.

Kernel thread scheduling is likely very different on UML than other
architectures.

My guess is that there could well be be gaps where no kernel thread is
scheduled (because another process is running), followed by resumption of a
thread other than the one which would have been resumed on other
virtualization.

--
]               Never tell me the odds!                 | ipv6 mesh networks [
]   Michael Richardson, Sandelman Software Works        | network architect  [
]     mcr@sandelman.ca  http://www.sandelman.ca/        |   ruby on rails    [

--
To unsubscribe from this list: send the line "unsubscribe linux-ext4" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Toralf Förster Sept. 7, 2013, 8:44 p.m. UTC | #8
On 08/28/2013 07:21 PM, Toralf Förster wrote:
> On 08/27/2013 08:06 PM, J. Bruce Fields wrote:
>> On Tue, Aug 13, 2013 at 05:53:14PM -0400, bfields wrote:
>>> On Mon, Aug 12, 2013 at 04:36:40PM +0200, Jan Kara wrote:
>>>> On Sun 11-08-13 11:48:49, Toralf Förster wrote:
>>>>> so that the server either crashes (if it is a user mode linux image) or at least its reboot functionality got broken
>>>>> - if the NFS server is hammered with scary NFS calls using a fuzzy tool running at a remote NFS client under a non-privileged user id.
>>>>>
>>>>> It can re reproduced, if
>>>>> 	- the NFS share is an EXT3 or EXT4 directory
>>>>> 	- and it is created at file located at tempfs and mounted via loop device
>>>>> 	- and the NFS server is forced to umount the NFS share
>>>>> 	- and the server forced to restart the NSF service afterwards
>>>>> 	- and trinity is used
>>>>>
>>>>> I could find a scenario for an automated bisect. 2 times it brought this commit 
>>>>> commit 68a3396178e6688ad7367202cdf0af8ed03c8727
>>>>> Author: J. Bruce Fields <bfields@redhat.com>
>>>>> Date:   Thu Mar 21 11:21:50 2013 -0400
>>>>>
>>>>>     nfsd4: shut down more of delegation earlier
>>>
>>> Thanks for the report.  I think I see the problem--after this commit
>>> nfs4_set_delegation() failures result in nfs4_put_delegation being
>>> called, but nfs4_put_delegation doesn't free the nfs4_file that has
>>> already been set by alloc_init_deleg().
>>>
>>> Let me think about how to fix that....
>>
>> Sorry for the slow response--can you check whether this fixes the
>> problem?
>>
> Yes.
> 
> With the attached patch the problem can't be reproduced any longer with
> the prepared test case and current git kernels.
> 
>> --b.
>>
>> commit 624a0ee0375940ce4aa36330b0b5a70af6d2b6f5
>> Author: J. Bruce Fields <bfields@redhat.com>
>> Date:   Thu Aug 15 16:55:26 2013 -0400
>>
>>     nfsd4: fix leak of inode reference on delegation failure
>>     
>>     This fixes a regression from 68a3396178e6688ad7367202cdf0af8ed03c8727
>>     "nfsd4: shut down more of delegation earlier".
>>     
>>     After that commit, nfs4_set_delegation() failures result in
>>     nfs4_put_delegation being called, but nfs4_put_delegation doesn't free
>>     the nfs4_file that has already been set by alloc_init_deleg().
>>     
>>     This can result in an oops on later unmounting the exported filesystem.
>>     
>>     Note also delaying the fi_had_conflict check we're able to return a
>>     better error (hence give 4.1 clients a better idea why the delegation
>>     failed; though note CONFLICT isn't an exact match here, as that's
>>     supposed to indicate a current conflict, but all we know here is that
>>     there was one recently).
>>     
>>     Reported-by: Toralf Förster <toralf.foerster@gmx.de>
>>     Signed-off-by: J. Bruce Fields <bfields@redhat.com>
>>
>> diff --git a/fs/nfsd/nfs4state.c b/fs/nfsd/nfs4state.c
>> index eb9cf81..0874998 100644
>> --- a/fs/nfsd/nfs4state.c
>> +++ b/fs/nfsd/nfs4state.c
>> @@ -368,11 +368,8 @@ static struct nfs4_delegation *
>>  alloc_init_deleg(struct nfs4_client *clp, struct nfs4_ol_stateid *stp, struct svc_fh *current_fh)
>>  {
>>  	struct nfs4_delegation *dp;
>> -	struct nfs4_file *fp = stp->st_file;
>>  
>>  	dprintk("NFSD alloc_init_deleg\n");
>> -	if (fp->fi_had_conflict)
>> -		return NULL;
>>  	if (num_delegations > max_delegations)
>>  		return NULL;
>>  	dp = delegstateid(nfs4_alloc_stid(clp, deleg_slab));
>> @@ -389,8 +386,7 @@ alloc_init_deleg(struct nfs4_client *clp, struct nfs4_ol_stateid *stp, struct sv
>>  	INIT_LIST_HEAD(&dp->dl_perfile);
>>  	INIT_LIST_HEAD(&dp->dl_perclnt);
>>  	INIT_LIST_HEAD(&dp->dl_recall_lru);
>> -	get_nfs4_file(fp);
>> -	dp->dl_file = fp;
>> +	dp->dl_file = NULL;
>>  	dp->dl_type = NFS4_OPEN_DELEGATE_READ;
>>  	fh_copy_shallow(&dp->dl_fh, &current_fh->fh_handle);
>>  	dp->dl_time = 0;
>> @@ -3044,22 +3040,35 @@ static int nfs4_setlease(struct nfs4_delegation *dp)
>>  	return 0;
>>  }
>>  
>> -static int nfs4_set_delegation(struct nfs4_delegation *dp)
>> +static int nfs4_set_delegation(struct nfs4_delegation *dp, struct nfs4_file *fp)
>>  {
>> -	struct nfs4_file *fp = dp->dl_file;
>> +	int status;
>>  
>> -	if (!fp->fi_lease)
>> -		return nfs4_setlease(dp);
>> +	if (fp->fi_had_conflict)
>> +		return -EAGAIN;
>> +	get_nfs4_file(fp);
>> +	dp->dl_file = fp;
>> +	if (!fp->fi_lease) {
>> +		status = nfs4_setlease(dp);
>> +		if (status)
>> +			goto out_free;
>> +		return 0;
>> +	}
>>  	spin_lock(&recall_lock);
>>  	if (fp->fi_had_conflict) {
>>  		spin_unlock(&recall_lock);
>> -		return -EAGAIN;
>> +		status = -EAGAIN;
>> +		goto out_free;
>>  	}
>>  	atomic_inc(&fp->fi_delegees);
>>  	list_add(&dp->dl_perfile, &fp->fi_delegations);
>>  	spin_unlock(&recall_lock);
>>  	list_add(&dp->dl_perclnt, &dp->dl_stid.sc_client->cl_delegations);
>>  	return 0;
>> +out_free:
>> +	put_nfs4_file(fp);
>> +	dp->dl_file = fp;
>> +	return status;
>>  }
>>  
>>  static void nfsd4_open_deleg_none_ext(struct nfsd4_open *open, int status)
>> @@ -3134,7 +3143,7 @@ nfs4_open_delegation(struct net *net, struct svc_fh *fh,
>>  	dp = alloc_init_deleg(oo->oo_owner.so_client, stp, fh);
>>  	if (dp == NULL)
>>  		goto out_no_deleg;
>> -	status = nfs4_set_delegation(dp);
>> +	status = nfs4_set_delegation(dp, stp->st_file);
>>  	if (status)
>>  		goto out_free;
>>  
>>
> 
> 

Today I run latest git tree with a patched UML (this patch + one for xterm issues) and got 2 times a core dump
when I fuzzy test an UML machine with a nearly identical scenario as already described but just shutdowned
both UML images instead of shooting one of it in the head.

I'll probably need time to figure out a test case, but just as a pre-info here's the back trace:

tfoerste@n22 ~ $ gdb --core=/mnt/ramdisk/core /usr/local/bin/linux-v3.11-7550-g768c9d3 -n -batch -ex bt

warning: core file may not match specified executable file.
[New LWP 7470]
[New LWP 7479]
[New LWP 7477]
[New LWP 7478]
Core was generated by `/usr/local/bin/linux-v3.11-7550-g768c9d3 earlyprintk ubda=/home/tfoerste/virtua'.
Program terminated with signal 6, Aborted.
#0  0xb77be424 in __kernel_vsyscall ()
#0  0xb77be424 in __kernel_vsyscall ()
#1  0x083aada5 in kill ()
#2  0x0807163d in uml_abort () at arch/um/os-Linux/util.c:93
#3  0x08071925 in os_dump_core () at arch/um/os-Linux/util.c:138
#4  0x080613a7 in panic_exit (self=0x85b1518 <panic_exit_notifier>, unused1=0, unused2=0x85e76e0 <buf.15920>) at arch/um/kernel/um_arch.c:240
#5  0x0809a398 in notifier_call_chain (nl=0x0, val=0, v=0x85e76e0 <buf.15920>, nr_to_call=-2, nr_calls=0x0) at kernel/notifier.c:93
#6  0x0809a4e3 in __atomic_notifier_call_chain (nr_calls=<optimized out>, nr_to_call=<optimized out>, v=<optimized out>, val=<optimized out>, nh=<optimized out>) at kernel/notifier.c:182
#7  atomic_notifier_call_chain (nh=0x85e76c4 <panic_notifier_list>, val=0, v=0x85e76e0 <buf.15920>) at kernel/notifier.c:191
#8  0x08408628 in panic (fmt=0x0) at kernel/panic.c:128
#9  0x081131c9 in shrink_dcache_for_umount_subtree (dentry=0x428028f0) at fs/dcache.c:941
#10 0x08113948 in shrink_dcache_for_umount (sb=0x463b8000) at fs/dcache.c:1002
#11 0x08101677 in generic_shutdown_super (sb=0x463b8000) at fs/super.c:404
#12 0x08102395 in kill_anon_super (sb=0x0) at fs/super.c:875
#13 0x081d3ff8 in nfs_kill_super (s=0x0) at fs/nfs/super.c:2598
#14 0x0810153a in deactivate_locked_super (s=0x463b8000) at fs/super.c:294
#15 0x081015d1 in deactivate_super (s=0x463b8000) at fs/super.c:319
#16 0x08119c0c in mntfree (mnt=<optimized out>) at fs/namespace.c:891
#17 mntput_no_expire (mnt=0x0) at fs/namespace.c:929
#18 0x0811b195 in SYSC_umount (flags=<optimized out>, name=<optimized out>) at fs/namespace.c:1335
#19 SyS_umount (name=134633856, flags=2) at fs/namespace.c:1305
#20 0x080618e2 in handle_syscall (r=0x498be5d4) at arch/um/kernel/skas/syscall.c:35
#21 0x08073c0d in handle_trap (local_using_sysemu=<optimized out>, regs=<optimized out>, pid=<optimized out>) at arch/um/os-Linux/skas/process.c:198
#22 userspace (regs=0x498be5d4) at arch/um/os-Linux/skas/process.c:431
#23 0x0805e65c in fork_handler () at arch/um/kernel/process.c:160
#24 0x00000000 in ?? ()
Richard Weinberger Sept. 7, 2013, 8:51 p.m. UTC | #9
On Sat, Sep 7, 2013 at 10:44 PM, Toralf Förster <toralf.foerster@gmx.de> wrote:
> Today I run latest git tree with a patched UML (this patch + one for xterm issues) and got 2 times a core dump
> when I fuzzy test an UML machine with a nearly identical scenario as already described but just shutdowned
> both UML images instead of shooting one of it in the head.
>
> I'll probably need time to figure out a test case, but just as a pre-info here's the back trace:
>
> tfoerste@n22 ~ $ gdb --core=/mnt/ramdisk/core /usr/local/bin/linux-v3.11-7550-g768c9d3 -n -batch -ex bt
>
> warning: core file may not match specified executable file.
> [New LWP 7470]
> [New LWP 7479]
> [New LWP 7477]
> [New LWP 7478]
> Core was generated by `/usr/local/bin/linux-v3.11-7550-g768c9d3 earlyprintk ubda=/home/tfoerste/virtua'.
> Program terminated with signal 6, Aborted.
> #0  0xb77be424 in __kernel_vsyscall ()
> #0  0xb77be424 in __kernel_vsyscall ()
> #1  0x083aada5 in kill ()
> #2  0x0807163d in uml_abort () at arch/um/os-Linux/util.c:93
> #3  0x08071925 in os_dump_core () at arch/um/os-Linux/util.c:138
> #4  0x080613a7 in panic_exit (self=0x85b1518 <panic_exit_notifier>, unused1=0, unused2=0x85e76e0 <buf.15920>) at arch/um/kernel/um_arch.c:240
> #5  0x0809a398 in notifier_call_chain (nl=0x0, val=0, v=0x85e76e0 <buf.15920>, nr_to_call=-2, nr_calls=0x0) at kernel/notifier.c:93
> #6  0x0809a4e3 in __atomic_notifier_call_chain (nr_calls=<optimized out>, nr_to_call=<optimized out>, v=<optimized out>, val=<optimized out>, nh=<optimized out>) at kernel/notifier.c:182
> #7  atomic_notifier_call_chain (nh=0x85e76c4 <panic_notifier_list>, val=0, v=0x85e76e0 <buf.15920>) at kernel/notifier.c:191
> #8  0x08408628 in panic (fmt=0x0) at kernel/panic.c:128
> #9  0x081131c9 in shrink_dcache_for_umount_subtree (dentry=0x428028f0) at fs/dcache.c:941

That is:
                        if (dentry->d_lockref.count != 0) {
                                printk(KERN_ERR
                                       "BUG: Dentry %p{i=%lx,n=%s}"
                                       " still in use (%d)"
                                       " [unmount of %s %s]\n",
                                       dentry,
                                       dentry->d_inode ?
                                       dentry->d_inode->i_ino : 0UL,
                                       dentry->d_name.name,
                                       dentry->d_lockref.count,
                                       dentry->d_sb->s_type->name,
                                       dentry->d_sb->s_id);
                                BUG();
                        }

Do you also have the console output?
J. Bruce Fields Sept. 10, 2013, 2:09 p.m. UTC | #10
On Sat, Sep 07, 2013 at 10:44:00PM +0200, Toralf Förster wrote:
> Today I run latest git tree with a patched UML (this patch + one for xterm issues) and got 2 times a core dump
> when I fuzzy test an UML machine with a nearly identical scenario as already described but just shutdowned
> both UML images instead of shooting one of it in the head.

This is a slightly different test case, so for now it sounds like this
could be a preexisting problem, not a regression?

--b.

> 
> I'll probably need time to figure out a test case, but just as a pre-info here's the back trace:
> 
> tfoerste@n22 ~ $ gdb --core=/mnt/ramdisk/core /usr/local/bin/linux-v3.11-7550-g768c9d3 -n -batch -ex bt
> 
> warning: core file may not match specified executable file.
> [New LWP 7470]
> [New LWP 7479]
> [New LWP 7477]
> [New LWP 7478]
> Core was generated by `/usr/local/bin/linux-v3.11-7550-g768c9d3 earlyprintk ubda=/home/tfoerste/virtua'.
> Program terminated with signal 6, Aborted.
> #0  0xb77be424 in __kernel_vsyscall ()
> #0  0xb77be424 in __kernel_vsyscall ()
> #1  0x083aada5 in kill ()
> #2  0x0807163d in uml_abort () at arch/um/os-Linux/util.c:93
> #3  0x08071925 in os_dump_core () at arch/um/os-Linux/util.c:138
> #4  0x080613a7 in panic_exit (self=0x85b1518 <panic_exit_notifier>, unused1=0, unused2=0x85e76e0 <buf.15920>) at arch/um/kernel/um_arch.c:240
> #5  0x0809a398 in notifier_call_chain (nl=0x0, val=0, v=0x85e76e0 <buf.15920>, nr_to_call=-2, nr_calls=0x0) at kernel/notifier.c:93
> #6  0x0809a4e3 in __atomic_notifier_call_chain (nr_calls=<optimized out>, nr_to_call=<optimized out>, v=<optimized out>, val=<optimized out>, nh=<optimized out>) at kernel/notifier.c:182
> #7  atomic_notifier_call_chain (nh=0x85e76c4 <panic_notifier_list>, val=0, v=0x85e76e0 <buf.15920>) at kernel/notifier.c:191
> #8  0x08408628 in panic (fmt=0x0) at kernel/panic.c:128
> #9  0x081131c9 in shrink_dcache_for_umount_subtree (dentry=0x428028f0) at fs/dcache.c:941
> #10 0x08113948 in shrink_dcache_for_umount (sb=0x463b8000) at fs/dcache.c:1002
> #11 0x08101677 in generic_shutdown_super (sb=0x463b8000) at fs/super.c:404
> #12 0x08102395 in kill_anon_super (sb=0x0) at fs/super.c:875
> #13 0x081d3ff8 in nfs_kill_super (s=0x0) at fs/nfs/super.c:2598
> #14 0x0810153a in deactivate_locked_super (s=0x463b8000) at fs/super.c:294
> #15 0x081015d1 in deactivate_super (s=0x463b8000) at fs/super.c:319
> #16 0x08119c0c in mntfree (mnt=<optimized out>) at fs/namespace.c:891
> #17 mntput_no_expire (mnt=0x0) at fs/namespace.c:929
> #18 0x0811b195 in SYSC_umount (flags=<optimized out>, name=<optimized out>) at fs/namespace.c:1335
> #19 SyS_umount (name=134633856, flags=2) at fs/namespace.c:1305
> #20 0x080618e2 in handle_syscall (r=0x498be5d4) at arch/um/kernel/skas/syscall.c:35
> #21 0x08073c0d in handle_trap (local_using_sysemu=<optimized out>, regs=<optimized out>, pid=<optimized out>) at arch/um/os-Linux/skas/process.c:198
> #22 userspace (regs=0x498be5d4) at arch/um/os-Linux/skas/process.c:431
> #23 0x0805e65c in fork_handler () at arch/um/kernel/process.c:160
> #24 0x00000000 in ?? ()
> 
> 
> 
> -- 
> MfG/Sincerely
> Toralf Förster
> pgp finger print: 7B1A 07F4 EC82 0F90 D4C2 8936 872A E508 7DB6 9DA3
--
To unsubscribe from this list: send the line "unsubscribe linux-ext4" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Toralf Förster Sept. 10, 2013, 3:51 p.m. UTC | #11
On 09/10/2013 04:09 PM, J. Bruce Fields wrote:
> On Sat, Sep 07, 2013 at 10:44:00PM +0200, Toralf Förster wrote:
>> Today I run latest git tree with a patched UML (this patch + one for xterm issues) and got 2 times a core dump
>> when I fuzzy test an UML machine with a nearly identical scenario as already described but just shutdowned
>> both UML images instead of shooting one of it in the head.
> 
> This is a slightly different test case, so for now it sounds like this
> could be a preexisting problem, not a regression?
> 
> --b.

If I found a test case I'll bisect it - but currently it does no longer
happen during normal operations w/ current git.#

(
Well, if I kill the NFS client during shutdown, then I get often a core
dump with the mentioned back trace but I think this doesn't count :-D
)

>>
>> I'll probably need time to figure out a test case, but just as a pre-info here's the back trace:
>>
>> tfoerste@n22 ~ $ gdb --core=/mnt/ramdisk/core /usr/local/bin/linux-v3.11-7550-g768c9d3 -n -batch -ex bt
>>
>> warning: core file may not match specified executable file.
>> [New LWP 7470]
>> [New LWP 7479]
>> [New LWP 7477]
>> [New LWP 7478]
>> Core was generated by `/usr/local/bin/linux-v3.11-7550-g768c9d3 earlyprintk ubda=/home/tfoerste/virtua'.
>> Program terminated with signal 6, Aborted.
>> #0  0xb77be424 in __kernel_vsyscall ()
>> #0  0xb77be424 in __kernel_vsyscall ()
>> #1  0x083aada5 in kill ()
>> #2  0x0807163d in uml_abort () at arch/um/os-Linux/util.c:93
>> #3  0x08071925 in os_dump_core () at arch/um/os-Linux/util.c:138
>> #4  0x080613a7 in panic_exit (self=0x85b1518 <panic_exit_notifier>, unused1=0, unused2=0x85e76e0 <buf.15920>) at arch/um/kernel/um_arch.c:240
>> #5  0x0809a398 in notifier_call_chain (nl=0x0, val=0, v=0x85e76e0 <buf.15920>, nr_to_call=-2, nr_calls=0x0) at kernel/notifier.c:93
>> #6  0x0809a4e3 in __atomic_notifier_call_chain (nr_calls=<optimized out>, nr_to_call=<optimized out>, v=<optimized out>, val=<optimized out>, nh=<optimized out>) at kernel/notifier.c:182
>> #7  atomic_notifier_call_chain (nh=0x85e76c4 <panic_notifier_list>, val=0, v=0x85e76e0 <buf.15920>) at kernel/notifier.c:191
>> #8  0x08408628 in panic (fmt=0x0) at kernel/panic.c:128
>> #9  0x081131c9 in shrink_dcache_for_umount_subtree (dentry=0x428028f0) at fs/dcache.c:941
>> #10 0x08113948 in shrink_dcache_for_umount (sb=0x463b8000) at fs/dcache.c:1002
>> #11 0x08101677 in generic_shutdown_super (sb=0x463b8000) at fs/super.c:404
>> #12 0x08102395 in kill_anon_super (sb=0x0) at fs/super.c:875
>> #13 0x081d3ff8 in nfs_kill_super (s=0x0) at fs/nfs/super.c:2598
>> #14 0x0810153a in deactivate_locked_super (s=0x463b8000) at fs/super.c:294
>> #15 0x081015d1 in deactivate_super (s=0x463b8000) at fs/super.c:319
>> #16 0x08119c0c in mntfree (mnt=<optimized out>) at fs/namespace.c:891
>> #17 mntput_no_expire (mnt=0x0) at fs/namespace.c:929
>> #18 0x0811b195 in SYSC_umount (flags=<optimized out>, name=<optimized out>) at fs/namespace.c:1335
>> #19 SyS_umount (name=134633856, flags=2) at fs/namespace.c:1305
>> #20 0x080618e2 in handle_syscall (r=0x498be5d4) at arch/um/kernel/skas/syscall.c:35
>> #21 0x08073c0d in handle_trap (local_using_sysemu=<optimized out>, regs=<optimized out>, pid=<optimized out>) at arch/um/os-Linux/skas/process.c:198
>> #22 userspace (regs=0x498be5d4) at arch/um/os-Linux/skas/process.c:431
>> #23 0x0805e65c in fork_handler () at arch/um/kernel/process.c:160
>> #24 0x00000000 in ?? ()
>>
>>
>>
>> -- 
>> MfG/Sincerely
>> Toralf Förster
>> pgp finger print: 7B1A 07F4 EC82 0F90 D4C2 8936 872A E508 7DB6 9DA3
>
Toralf Förster Sept. 22, 2013, 4:58 p.m. UTC | #12
On 09/10/2013 04:09 PM, J. Bruce Fields wrote:
> On Sat, Sep 07, 2013 at 10:44:00PM +0200, Toralf Förster wrote:
>> Today I run latest git tree with a patched UML (this patch + one for xterm issues) and got 2 times a core dump
>> when I fuzzy test an UML machine with a nearly identical scenario as already described but just shutdowned
>> both UML images instead of shooting one of it in the head.
> 
> This is a slightly different test case, so for now it sounds like this
> could be a preexisting problem, not a regression?
> 
Yes, pre-existing. I'm trying to bisect it, the issue is already in 7d3107d.

But every attempt to get the first bad commit before that commit id
failed till now. I started many attempts between v3.10 and 7d3107d. I
could not reproduce that issue 100% in that interval. In that interval
either all tested commits doesn't show the issue or the NFS client hangs
during the stop of NFS service infinitely - and counting that as a "bad"
commit doesn't work.

FWIW a single test case currently takes 2 1/2 hour and even then I do
not fully trust the result  (except for a bad commit, that's clear.


> --b.
> 
>>
>> I'll probably need time to figure out a test case, but just as a pre-info here's the back trace:
>>
>> tfoerste@n22 ~ $ gdb --core=/mnt/ramdisk/core /usr/local/bin/linux-v3.11-7550-g768c9d3 -n -batch -ex bt
>>
>> warning: core file may not match specified executable file.
>> [New LWP 7470]
>> [New LWP 7479]
>> [New LWP 7477]
>> [New LWP 7478]
>> Core was generated by `/usr/local/bin/linux-v3.11-7550-g768c9d3 earlyprintk ubda=/home/tfoerste/virtua'.
>> Program terminated with signal 6, Aborted.
>> #0  0xb77be424 in __kernel_vsyscall ()
>> #0  0xb77be424 in __kernel_vsyscall ()
>> #1  0x083aada5 in kill ()
>> #2  0x0807163d in uml_abort () at arch/um/os-Linux/util.c:93
>> #3  0x08071925 in os_dump_core () at arch/um/os-Linux/util.c:138
>> #4  0x080613a7 in panic_exit (self=0x85b1518 <panic_exit_notifier>, unused1=0, unused2=0x85e76e0 <buf.15920>) at arch/um/kernel/um_arch.c:240
>> #5  0x0809a398 in notifier_call_chain (nl=0x0, val=0, v=0x85e76e0 <buf.15920>, nr_to_call=-2, nr_calls=0x0) at kernel/notifier.c:93
>> #6  0x0809a4e3 in __atomic_notifier_call_chain (nr_calls=<optimized out>, nr_to_call=<optimized out>, v=<optimized out>, val=<optimized out>, nh=<optimized out>) at kernel/notifier.c:182
>> #7  atomic_notifier_call_chain (nh=0x85e76c4 <panic_notifier_list>, val=0, v=0x85e76e0 <buf.15920>) at kernel/notifier.c:191
>> #8  0x08408628 in panic (fmt=0x0) at kernel/panic.c:128
>> #9  0x081131c9 in shrink_dcache_for_umount_subtree (dentry=0x428028f0) at fs/dcache.c:941
>> #10 0x08113948 in shrink_dcache_for_umount (sb=0x463b8000) at fs/dcache.c:1002
>> #11 0x08101677 in generic_shutdown_super (sb=0x463b8000) at fs/super.c:404
>> #12 0x08102395 in kill_anon_super (sb=0x0) at fs/super.c:875
>> #13 0x081d3ff8 in nfs_kill_super (s=0x0) at fs/nfs/super.c:2598
>> #14 0x0810153a in deactivate_locked_super (s=0x463b8000) at fs/super.c:294
>> #15 0x081015d1 in deactivate_super (s=0x463b8000) at fs/super.c:319
>> #16 0x08119c0c in mntfree (mnt=<optimized out>) at fs/namespace.c:891
>> #17 mntput_no_expire (mnt=0x0) at fs/namespace.c:929
>> #18 0x0811b195 in SYSC_umount (flags=<optimized out>, name=<optimized out>) at fs/namespace.c:1335
>> #19 SyS_umount (name=134633856, flags=2) at fs/namespace.c:1305
>> #20 0x080618e2 in handle_syscall (r=0x498be5d4) at arch/um/kernel/skas/syscall.c:35
>> #21 0x08073c0d in handle_trap (local_using_sysemu=<optimized out>, regs=<optimized out>, pid=<optimized out>) at arch/um/os-Linux/skas/process.c:198
>> #22 userspace (regs=0x498be5d4) at arch/um/os-Linux/skas/process.c:431
>> #23 0x0805e65c in fork_handler () at arch/um/kernel/process.c:160
>> #24 0x00000000 in ?? ()
>>
>>
>>
>> -- 
>> MfG/Sincerely
>> Toralf Förster
>> pgp finger print: 7B1A 07F4 EC82 0F90 D4C2 8936 872A E508 7DB6 9DA3
>
J. Bruce Fields Sept. 23, 2013, 5:41 p.m. UTC | #13
On Sun, Sep 22, 2013 at 06:58:29PM +0200, Toralf Förster wrote:
> On 09/10/2013 04:09 PM, J. Bruce Fields wrote:
> > On Sat, Sep 07, 2013 at 10:44:00PM +0200, Toralf Förster wrote:
> >> Today I run latest git tree with a patched UML (this patch + one for xterm issues) and got 2 times a core dump
> >> when I fuzzy test an UML machine with a nearly identical scenario as already described but just shutdowned
> >> both UML images instead of shooting one of it in the head.
> > 
> > This is a slightly different test case, so for now it sounds like this
> > could be a preexisting problem, not a regression?
> > 
> Yes, pre-existing. I'm trying to bisect it, the issue is already in 7d3107d.
> 
> But every attempt to get the first bad commit before that commit id
> failed till now. I started many attempts between v3.10 and 7d3107d. I
> could not reproduce that issue 100% in that interval. In that interval
> either all tested commits doesn't show the issue or the NFS client hangs
> during the stop of NFS service infinitely - and counting that as a "bad"
> commit doesn't work.
> 
> FWIW a single test case currently takes 2 1/2 hour and even then I do
> not fully trust the result  (except for a bad commit, that's clear.

Well, we should figure out some other way to narrow down the problem....

I was trying to at least work out where exactly this crash was, but I
don't have a commit 768c9d3.

--b.

> 
> 
> > --b.
> > 
> >>
> >> I'll probably need time to figure out a test case, but just as a pre-info here's the back trace:
> >>
> >> tfoerste@n22 ~ $ gdb --core=/mnt/ramdisk/core /usr/local/bin/linux-v3.11-7550-g768c9d3 -n -batch -ex bt
> >>
> >> warning: core file may not match specified executable file.
> >> [New LWP 7470]
> >> [New LWP 7479]
> >> [New LWP 7477]
> >> [New LWP 7478]
> >> Core was generated by `/usr/local/bin/linux-v3.11-7550-g768c9d3 earlyprintk ubda=/home/tfoerste/virtua'.
> >> Program terminated with signal 6, Aborted.
> >> #0  0xb77be424 in __kernel_vsyscall ()
> >> #0  0xb77be424 in __kernel_vsyscall ()
> >> #1  0x083aada5 in kill ()
> >> #2  0x0807163d in uml_abort () at arch/um/os-Linux/util.c:93
> >> #3  0x08071925 in os_dump_core () at arch/um/os-Linux/util.c:138
> >> #4  0x080613a7 in panic_exit (self=0x85b1518 <panic_exit_notifier>, unused1=0, unused2=0x85e76e0 <buf.15920>) at arch/um/kernel/um_arch.c:240
> >> #5  0x0809a398 in notifier_call_chain (nl=0x0, val=0, v=0x85e76e0 <buf.15920>, nr_to_call=-2, nr_calls=0x0) at kernel/notifier.c:93
> >> #6  0x0809a4e3 in __atomic_notifier_call_chain (nr_calls=<optimized out>, nr_to_call=<optimized out>, v=<optimized out>, val=<optimized out>, nh=<optimized out>) at kernel/notifier.c:182
> >> #7  atomic_notifier_call_chain (nh=0x85e76c4 <panic_notifier_list>, val=0, v=0x85e76e0 <buf.15920>) at kernel/notifier.c:191
> >> #8  0x08408628 in panic (fmt=0x0) at kernel/panic.c:128
> >> #9  0x081131c9 in shrink_dcache_for_umount_subtree (dentry=0x428028f0) at fs/dcache.c:941
> >> #10 0x08113948 in shrink_dcache_for_umount (sb=0x463b8000) at fs/dcache.c:1002
> >> #11 0x08101677 in generic_shutdown_super (sb=0x463b8000) at fs/super.c:404
> >> #12 0x08102395 in kill_anon_super (sb=0x0) at fs/super.c:875
> >> #13 0x081d3ff8 in nfs_kill_super (s=0x0) at fs/nfs/super.c:2598
> >> #14 0x0810153a in deactivate_locked_super (s=0x463b8000) at fs/super.c:294
> >> #15 0x081015d1 in deactivate_super (s=0x463b8000) at fs/super.c:319
> >> #16 0x08119c0c in mntfree (mnt=<optimized out>) at fs/namespace.c:891
> >> #17 mntput_no_expire (mnt=0x0) at fs/namespace.c:929
> >> #18 0x0811b195 in SYSC_umount (flags=<optimized out>, name=<optimized out>) at fs/namespace.c:1335
> >> #19 SyS_umount (name=134633856, flags=2) at fs/namespace.c:1305
> >> #20 0x080618e2 in handle_syscall (r=0x498be5d4) at arch/um/kernel/skas/syscall.c:35
> >> #21 0x08073c0d in handle_trap (local_using_sysemu=<optimized out>, regs=<optimized out>, pid=<optimized out>) at arch/um/os-Linux/skas/process.c:198
> >> #22 userspace (regs=0x498be5d4) at arch/um/os-Linux/skas/process.c:431
> >> #23 0x0805e65c in fork_handler () at arch/um/kernel/process.c:160
> >> #24 0x00000000 in ?? ()
> >>
> >>
> >>
> >> -- 
> >> MfG/Sincerely
> >> Toralf Förster
> >> pgp finger print: 7B1A 07F4 EC82 0F90 D4C2 8936 872A E508 7DB6 9DA3
> > 
> 
> 
> -- 
> MfG/Sincerely
> Toralf Förster
> pgp finger print: 7B1A 07F4 EC82 0F90 D4C2 8936 872A E508 7DB6 9DA3
> --
> 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-ext4" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Toralf Förster Oct. 2, 2013, 8:29 p.m. UTC | #14
On 09/23/2013 07:41 PM, J. Bruce Fields wrote:
> On Sun, Sep 22, 2013 at 06:58:29PM +0200, Toralf Förster wrote:
>> On 09/10/2013 04:09 PM, J. Bruce Fields wrote:
>>> On Sat, Sep 07, 2013 at 10:44:00PM +0200, Toralf Förster wrote:
>>>> Today I run latest git tree with a patched UML (this patch + one for xterm issues) and got 2 times a core dump
>>>> when I fuzzy test an UML machine with a nearly identical scenario as already described but just shutdowned
>>>> both UML images instead of shooting one of it in the head.
>>>
>>> This is a slightly different test case, so for now it sounds like this
>>> could be a preexisting problem, not a regression?
>>>
>> Yes, pre-existing. I'm trying to bisect it, the issue is already in 7d3107d.
>>
>> But every attempt to get the first bad commit before that commit id
>> failed till now. I started many attempts between v3.10 and 7d3107d. I
>> could not reproduce that issue 100% in that interval. In that interval
>> either all tested commits doesn't show the issue or the NFS client hangs
>> during the stop of NFS service infinitely - and counting that as a "bad"
>> commit doesn't work.
>>
>> FWIW a single test case currently takes 2 1/2 hour and even then I do
>> not fully trust the result  (except for a bad commit, that's clear.
> 
> Well, we should figure out some other way to narrow down the problem....
> 
> I was trying to at least work out where exactly this crash was, but I
> don't have a commit 768c9d3.
> 
> --b.
> 

just for completeness of this thread, that issue could be bisected in
the mean while :

http://thread.gmane.org/gmane.linux.kernel/1569818

>>
>>
>>> --b.
>>>
>>>>
>>>> I'll probably need time to figure out a test case, but just as a pre-info here's the back trace:
>>>>
>>>> tfoerste@n22 ~ $ gdb --core=/mnt/ramdisk/core /usr/local/bin/linux-v3.11-7550-g768c9d3 -n -batch -ex bt
>>>>
>>>> warning: core file may not match specified executable file.
>>>> [New LWP 7470]
>>>> [New LWP 7479]
>>>> [New LWP 7477]
>>>> [New LWP 7478]
>>>> Core was generated by `/usr/local/bin/linux-v3.11-7550-g768c9d3 earlyprintk ubda=/home/tfoerste/virtua'.
>>>> Program terminated with signal 6, Aborted.
>>>> #0  0xb77be424 in __kernel_vsyscall ()
>>>> #0  0xb77be424 in __kernel_vsyscall ()
>>>> #1  0x083aada5 in kill ()
>>>> #2  0x0807163d in uml_abort () at arch/um/os-Linux/util.c:93
>>>> #3  0x08071925 in os_dump_core () at arch/um/os-Linux/util.c:138
>>>> #4  0x080613a7 in panic_exit (self=0x85b1518 <panic_exit_notifier>, unused1=0, unused2=0x85e76e0 <buf.15920>) at arch/um/kernel/um_arch.c:240
>>>> #5  0x0809a398 in notifier_call_chain (nl=0x0, val=0, v=0x85e76e0 <buf.15920>, nr_to_call=-2, nr_calls=0x0) at kernel/notifier.c:93
>>>> #6  0x0809a4e3 in __atomic_notifier_call_chain (nr_calls=<optimized out>, nr_to_call=<optimized out>, v=<optimized out>, val=<optimized out>, nh=<optimized out>) at kernel/notifier.c:182
>>>> #7  atomic_notifier_call_chain (nh=0x85e76c4 <panic_notifier_list>, val=0, v=0x85e76e0 <buf.15920>) at kernel/notifier.c:191
>>>> #8  0x08408628 in panic (fmt=0x0) at kernel/panic.c:128
>>>> #9  0x081131c9 in shrink_dcache_for_umount_subtree (dentry=0x428028f0) at fs/dcache.c:941
>>>> #10 0x08113948 in shrink_dcache_for_umount (sb=0x463b8000) at fs/dcache.c:1002
>>>> #11 0x08101677 in generic_shutdown_super (sb=0x463b8000) at fs/super.c:404
>>>> #12 0x08102395 in kill_anon_super (sb=0x0) at fs/super.c:875
>>>> #13 0x081d3ff8 in nfs_kill_super (s=0x0) at fs/nfs/super.c:2598
>>>> #14 0x0810153a in deactivate_locked_super (s=0x463b8000) at fs/super.c:294
>>>> #15 0x081015d1 in deactivate_super (s=0x463b8000) at fs/super.c:319
>>>> #16 0x08119c0c in mntfree (mnt=<optimized out>) at fs/namespace.c:891
>>>> #17 mntput_no_expire (mnt=0x0) at fs/namespace.c:929
>>>> #18 0x0811b195 in SYSC_umount (flags=<optimized out>, name=<optimized out>) at fs/namespace.c:1335
>>>> #19 SyS_umount (name=134633856, flags=2) at fs/namespace.c:1305
>>>> #20 0x080618e2 in handle_syscall (r=0x498be5d4) at arch/um/kernel/skas/syscall.c:35
>>>> #21 0x08073c0d in handle_trap (local_using_sysemu=<optimized out>, regs=<optimized out>, pid=<optimized out>) at arch/um/os-Linux/skas/process.c:198
>>>> #22 userspace (regs=0x498be5d4) at arch/um/os-Linux/skas/process.c:431
>>>> #23 0x0805e65c in fork_handler () at arch/um/kernel/process.c:160
>>>> #24 0x00000000 in ?? ()
>>>>
>>>>
>>>>
>>>> -- 
>>>> MfG/Sincerely
>>>> Toralf Förster
>>>> pgp finger print: 7B1A 07F4 EC82 0F90 D4C2 8936 872A E508 7DB6 9DA3
>>>
>>
>>
>> -- 
>> MfG/Sincerely
>> Toralf Förster
>> pgp finger print: 7B1A 07F4 EC82 0F90 D4C2 8936 872A E508 7DB6 9DA3
>> --
>> 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
>
diff mbox

Patch

diff --git a/fs/nfsd/nfs4state.c b/fs/nfsd/nfs4state.c
index eb9cf81..0874998 100644
--- a/fs/nfsd/nfs4state.c
+++ b/fs/nfsd/nfs4state.c
@@ -368,11 +368,8 @@  static struct nfs4_delegation *
 alloc_init_deleg(struct nfs4_client *clp, struct nfs4_ol_stateid *stp, struct svc_fh *current_fh)
 {
 	struct nfs4_delegation *dp;
-	struct nfs4_file *fp = stp->st_file;
 
 	dprintk("NFSD alloc_init_deleg\n");
-	if (fp->fi_had_conflict)
-		return NULL;
 	if (num_delegations > max_delegations)
 		return NULL;
 	dp = delegstateid(nfs4_alloc_stid(clp, deleg_slab));
@@ -389,8 +386,7 @@  alloc_init_deleg(struct nfs4_client *clp, struct nfs4_ol_stateid *stp, struct sv
 	INIT_LIST_HEAD(&dp->dl_perfile);
 	INIT_LIST_HEAD(&dp->dl_perclnt);
 	INIT_LIST_HEAD(&dp->dl_recall_lru);
-	get_nfs4_file(fp);
-	dp->dl_file = fp;
+	dp->dl_file = NULL;
 	dp->dl_type = NFS4_OPEN_DELEGATE_READ;
 	fh_copy_shallow(&dp->dl_fh, &current_fh->fh_handle);
 	dp->dl_time = 0;
@@ -3044,22 +3040,35 @@  static int nfs4_setlease(struct nfs4_delegation *dp)
 	return 0;
 }
 
-static int nfs4_set_delegation(struct nfs4_delegation *dp)
+static int nfs4_set_delegation(struct nfs4_delegation *dp, struct nfs4_file *fp)
 {
-	struct nfs4_file *fp = dp->dl_file;
+	int status;
 
-	if (!fp->fi_lease)
-		return nfs4_setlease(dp);
+	if (fp->fi_had_conflict)
+		return -EAGAIN;
+	get_nfs4_file(fp);
+	dp->dl_file = fp;
+	if (!fp->fi_lease) {
+		status = nfs4_setlease(dp);
+		if (status)
+			goto out_free;
+		return 0;
+	}
 	spin_lock(&recall_lock);
 	if (fp->fi_had_conflict) {
 		spin_unlock(&recall_lock);
-		return -EAGAIN;
+		status = -EAGAIN;
+		goto out_free;
 	}
 	atomic_inc(&fp->fi_delegees);
 	list_add(&dp->dl_perfile, &fp->fi_delegations);
 	spin_unlock(&recall_lock);
 	list_add(&dp->dl_perclnt, &dp->dl_stid.sc_client->cl_delegations);
 	return 0;
+out_free:
+	put_nfs4_file(fp);
+	dp->dl_file = fp;
+	return status;
 }
 
 static void nfsd4_open_deleg_none_ext(struct nfsd4_open *open, int status)
@@ -3134,7 +3143,7 @@  nfs4_open_delegation(struct net *net, struct svc_fh *fh,
 	dp = alloc_init_deleg(oo->oo_owner.so_client, stp, fh);
 	if (dp == NULL)
 		goto out_no_deleg;
-	status = nfs4_set_delegation(dp);
+	status = nfs4_set_delegation(dp, stp->st_file);
 	if (status)
 		goto out_free;