From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1751616AbcFCAo6 (ORCPT ); Thu, 2 Jun 2016 20:44:58 -0400 Received: from us-smtp-delivery-194.mimecast.com ([216.205.24.194]:57778 "EHLO us-smtp-delivery-194.mimecast.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1750880AbcFCAoz (ORCPT ); Thu, 2 Jun 2016 20:44:55 -0400 From: Trond Myklebust To: Oleg Drokin , Al Viro , "J. Bruce Fields" CC: "linux-nfs@vger.kernel.org" , " Mailing List" , "" Subject: Re: NFS/d_splice_alias breakage Thread-Topic: NFS/d_splice_alias breakage Thread-Index: AQHRvSMFbML/GIdeYEW3Nf/vX/fvSZ/WpO4A Date: Fri, 3 Jun 2016 00:44:51 +0000 Message-ID: References: In-Reply-To: Accept-Language: en-US Content-Language: en-US X-MS-Has-Attach: X-MS-TNEF-Correlator: x-ms-exchange-messagesentrepresentingtype: 1 x-originating-ip: [68.49.162.121] x-ms-office365-filtering-correlation-id: 18b15009-5c0f-4803-38fc-08d38b4845c6 x-microsoft-exchange-diagnostics: 1;BN3PR11MB0306;5:QQqW8v6oc7No2/8Ho9WMfKoWMogL4Wb93fHLETon0N+6b9E9EJ8UDDRsdB9jfzS34h7Nyp8K2eix9Lsv/3MivxNELJB6nej3iUHXUfdDQbu7XvASX8ppxCNtbTvfVvjBayKnXLTo6Mohg0vMxrDDcQ==;24:S3aHaOKBvo/fpxpkx/KP/TN/NG2Uwdfww8I4GQ39OgNlxZPmclXIc3KdETu5K1qnGvfmU0GvNwtBESKx9J27BtrVc3SC9rwmTD1wpUAJFQg=;7:2oKB5VGmribNwd0LO41AeOuJg6DvIuqNE+1o+MRJ6qii3IgzzweFuvo0Jh1PPHLotX6+mk5rvXxmn9cC28ZiUNNLUIPLCmjTHA5a5lSfkR6yl7D273w3RZKtBRuMuoGk+jBuw7Kd8baVxjupoYe966RjA7a4rTzmqp/iwpFepZWO6aAHnjMOtUFMTQuPmMjVlwhgK+HnMbqfpZGjRLpSsh5mqRfUyjRl8bW9ECtyaI0=;20:V/UpSnnqGx4pZy7nf8iOH6t364CqGvsgiKRspmd/tC43p7PgYZET2030p/jXiyBy63Uxj5PwD2+JjJxu6CpCKFOSEtOEhUfHw2l0pkCM/pXftTH4qtvymup1JlK+RG7EtIOSD+Ajf17Qau7wjs82Lcq3881Pi46NB/+MzCA+HHM= x-microsoft-antispam: UriScan:;BCL:0;PCL:0;RULEID:;SRVR:BN3PR11MB0306; x-microsoft-antispam-prvs: x-exchange-antispam-report-test: UriScan:(9452136761055); x-exchange-antispam-report-cfa-test: BCL:0;PCL:0;RULEID:(6040130)(601004)(2401047)(8121501046)(5005006)(3002001)(10201501046)(6041072)(6043046);SRVR:BN3PR11MB0306;BCL:0;PCL:0;RULEID:;SRVR:BN3PR11MB0306; x-forefront-prvs: 0962D394D2 x-forefront-antispam-report: SFV:NSPM;SFS:(10019020)(6009001)(24454002)(77096005)(4477795004)(122556002)(86362001)(106116001)(66066001)(575784001)(33656002)(11100500001)(92566002)(19580405001)(2950100001)(81166006)(76176999)(2900100001)(8676002)(5004730100002)(54356999)(5002640100001)(10400500002)(19580395003)(8936002)(50986999)(3280700002)(82746002)(5008740100001)(3660700001)(2906002)(4326007)(99286002)(6116002)(83716003)(5001770100001)(586003)(102836003)(3846002)(189998001)(87936001)(36756003);DIR:OUT;SFP:1102;SCL:1;SRVR:BN3PR11MB0306;H:BN3PR11MB0305.namprd11.prod.outlook.com;FPR:;SPF:None;MLV:sfv;LANG:en; spamdiagnosticoutput: 1:23 spamdiagnosticmetadata: NSPM Content-ID: <4DD310E8BF26594C8477C84C7C4E9F88@namprd11.prod.outlook.com> MIME-Version: 1.0 X-OriginatorOrg: primarydata.com X-MS-Exchange-CrossTenant-originalarrivaltime: 03 Jun 2016 00:44:51.2959 (UTC) X-MS-Exchange-CrossTenant-fromentityheader: Hosted X-MS-Exchange-CrossTenant-id: 03193ed6-8726-4bb3-a832-18ab0d28adb7 X-MS-Exchange-Transport-CrossTenantHeadersStamped: BN3PR11MB0306 X-MC-Unique: r1PEA4G8RsSh10HrLV9Tfw-1 Content-Type: text/plain; charset=UTF-8 Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Transfer-Encoding: 8bit X-MIME-Autoconverted: from base64 to 8bit by mail.home.local id u530j5us024682 On 6/2/16, 18:46, "linux-nfs-owner@vger.kernel.org on behalf of Oleg Drokin" wrote: >Hello! > > I just came across a bug (trying to run some Lustre test scripts against NFS, while hunting for another nfsd bug) > that seems to be present since at least 2014 that lets users crash nfs client locally. > > Here's some interesting comment quote first from d_obtain_alias: > >> * Cluster filesystems may call this function with a negative, hashed dentry. >> * In that case, we know that the inode will be a regular file, and also this >> * will only occur during atomic_open. So we need to check for the dentry >> * being already hashed only in the final case. >> */ >> struct dentry *d_splice_alias(struct inode *inode, struct dentry *dentry) >> { >> if (IS_ERR(inode)) >> return ERR_CAST(inode); >> >> BUG_ON(!d_unhashed(dentry)); > ^^^^^^^^^^^^^^ - This does not align well with the quote above. > >It got imported here by commit b5ae6b15bd73e35b129408755a0804287a87e041 > >Removing the BUG_ON headon is not going to work since the d_rehash of old >is now folded into __d_add and we might not want to move that condition there. > >doing an >if (d_unhashed(dentry)) > __d_add(dentry, inode); >else > d_instantiate(dentry, inode); > >also does not look super pretty and who knows how all of the previous code >like _d_find_any_alias would react. > >Al, I think you might want to chime in here on how to better handle this? > >The problem was there at least since 3.10 it appears where the fs/nfs/dir.c code >was calling d_materialise_unique() that did require the dentry to be unhashed. > >Not sure how this was not hit earlier. The crash looks like this (I added >a printk to ensure this is what is going on indeed and not some other weird race): > >[ 64.489326] Calling into d_splice_alias with hashed dentry, dentry->d_inode (null) inode ffff88010f500c70 >[ 64.489549] ------------[ cut here ]------------ >[ 64.489642] kernel BUG at /home/green/bk/linux/fs/dcache.c:2989! >[ 64.489750] invalid opcode: 0000 [#1] SMP DEBUG_PAGEALLOC >[ 64.489853] Modules linked in: loop rpcsec_gss_krb5 joydev pcspkr acpi_cpufreq i2c_piix4 tpm_tis tpm nfsd drm_kms_helper ttm drm serio_raw virtio_blk >[ 64.491111] CPU: 6 PID: 7125 Comm: file_concat.sh Not tainted 4.7.0-rc1-vm-nfs+ #99 >[ 64.492069] Hardware name: Bochs Bochs, BIOS Bochs 01/01/2011 >[ 64.492489] task: ffff880110a801c0 ti: ffff8800c283c000 task.ti: ffff8800c283c000 >[ 64.493159] RIP: 0010:[] [] d_splice_alias+0x31f/0x480 >[ 64.493866] RSP: 0018:ffff8800c283fb20 EFLAGS: 00010282 >[ 64.494238] RAX: 0000000000000067 RBX: ffff88010f500c70 RCX: 0000000000000000 >[ 64.494625] RDX: 0000000000000067 RSI: ffff8800d08d2ed0 RDI: ffff88010f500c70 >[ 64.495021] RBP: ffff8800c283fb78 R08: 0000000000000001 R09: 0000000000000000 >[ 64.495407] R10: 0000000000000001 R11: 0000000000000001 R12: ffff8800d08d2ed0 >[ 64.495804] R13: 0000000000000000 R14: ffff8800d4a56f00 R15: ffff8800d0bb8c70 >[ 64.496199] FS: 00007f94ae25c700(0000) GS:ffff88011f580000(0000) knlGS:0000000000000000 >[ 64.496859] CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033 >[ 64.497235] CR2: 000055e5d3fb46a4 CR3: 00000000ce364000 CR4: 00000000000006e0 >[ 64.497626] Stack: >[ 64.497961] ffffffff8132154e ffff8800c283fb68 ffffffff81325916 0000000000000000 >[ 64.498765] 0000000000000000 ffff8801109459c0 0000000000000000 ffff8800d0bb8c70 >[ 64.499578] ffff8800c283fba0 ffff8800d08d2ed0 ffff8800cb927080 ffff8800c283fc18 >[ 64.500385] Call Trace: >[ 64.500727] [] ? nfs_lookup+0x17e/0x320 >[ 64.501103] [] ? __put_nfs_open_context+0xc6/0xf0 >[ 64.501477] [] nfs_atomic_open+0x8b/0x430 >[ 64.501850] [] lookup_open+0x29f/0x7a0 >[ 64.502222] [] path_openat+0x4de/0xfc0 >[ 64.502591] [] do_filp_open+0x7e/0xe0 >[ 64.502964] [] ? __alloc_fd+0xbc/0x170 >[ 64.503347] [] ? _raw_spin_unlock+0x27/0x40 >[ 64.503719] [] ? __alloc_fd+0xbc/0x170 >[ 64.504097] [] do_sys_open+0x116/0x1f0 >[ 64.504465] [] SyS_open+0x1e/0x20 >[ 64.504831] [] entry_SYSCALL_64_fastpath+0x1e/0xad >[ 64.505218] Code: 01 e8 46 20 5b 00 85 db 74 0b 4c 89 ff 4c 63 fb e8 87 d8 ff ff 4c 89 e7 e8 2f 3c 00 00 4c 89 f8 e9 5e fe ff ff 0f 0b 48 89 f8 c3 <0f> 0b 48 8b 43 40 4c 8b 78 58 49 8d 8f 58 03 00 00 eb 02 f3 90 >[ 64.508754] RIP [] d_splice_alias+0x31f/0x480 >[ 64.509176] RSP That would have to be a really tight race, since the code in _nfs4_open_and_get_state() currently reads: d_drop(dentry); alias = d_exact_alias(dentry, state->inode); if (!alias) alias = d_splice_alias(igrab(state->inode), dentry); IOW: something would have to be acting between the d_drop() and d_splice_alias() above... Al, I’ve been distracted by personal matters in the past 6 months. What is there to guarantee exclusion of the readdirplus dentry instantiation and the NFSv4 atomic open in the brave new world of VFS, June 2016 vintage? Cheers Trond