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@xxxxxxxxxx>
>>>>> 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@xxxxxxxxxx>
>> 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@xxxxxx>
>> Signed-off-by: J. Bruce Fields <bfields@xxxxxxxxxx>
>>
>> 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, ¤t_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 ?? ()
--
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@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
