From patchwork Thu Mar 14 18:08:54 2019 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Steve French X-Patchwork-Id: 10853367 Return-Path: Received: from mail.wl.linuxfoundation.org (pdx-wl-mail.web.codeaurora.org [172.30.200.125]) by pdx-korg-patchwork-2.web.codeaurora.org (Postfix) with ESMTP id AA1886C2 for ; Thu, 14 Mar 2019 18:09:18 +0000 (UTC) Received: from mail.wl.linuxfoundation.org (localhost [127.0.0.1]) by mail.wl.linuxfoundation.org (Postfix) with ESMTP id 96A1D28FA8 for ; Thu, 14 Mar 2019 18:09:18 +0000 (UTC) Received: by mail.wl.linuxfoundation.org (Postfix, from userid 486) id 8904D29084; Thu, 14 Mar 2019 18:09:18 +0000 (UTC) X-Spam-Checker-Version: SpamAssassin 3.3.1 (2010-03-16) on pdx-wl-mail.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-8.0 required=2.0 tests=BAYES_00,DKIM_SIGNED, DKIM_VALID,DKIM_VALID_AU,FREEMAIL_FROM,MAILING_LIST_MULTI,RCVD_IN_DNSWL_HI autolearn=ham version=3.3.1 Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by mail.wl.linuxfoundation.org (Postfix) with ESMTP id DB5AE28FA8 for ; Thu, 14 Mar 2019 18:09:17 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1727349AbfCNSJI (ORCPT ); Thu, 14 Mar 2019 14:09:08 -0400 Received: from mail-pf1-f195.google.com ([209.85.210.195]:36967 "EHLO mail-pf1-f195.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1726531AbfCNSJH (ORCPT ); Thu, 14 Mar 2019 14:09:07 -0400 Received: by mail-pf1-f195.google.com with SMTP id 8so964722pfr.4 for ; Thu, 14 Mar 2019 11:09:07 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20161025; h=mime-version:references:in-reply-to:from:date:message-id:subject:to; bh=nEo9jtdnWK4RSL1N6Mtdsig8ikwU/Y0pMwxTSS4HQus=; b=tMM9+guRHRWsAWhaEFfrW1GyieGW2px1W7wUKAKtTbWtp4rUYhguzwLkT6HS4Z3akm 7dlBS05vP7dook5/vPS4baXVSzto25lQ5LCF9qxwnP3EfUQD5WY1tjfzN+Vt8UzFBh9E 6YAba+mhDHbH6oykPPnyReHXRjSDGPzI2O1K7EmB9Uu0G6qIMxqgfpEX9IXnUv9bbR11 1mVONgSyxGryhNv3xGcwRNFe8thDRZz8hvdSyIArBC0mVHfkBX4nPBRRVA4BrFgLTELQ 5FPPy9N90DkPlUhkTFWaFj8ThPmDtwvdV3eF71fHGYtqmsq7ceVxfx/mGjZpALxFBYAI mCLg== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:mime-version:references:in-reply-to:from:date :message-id:subject:to; bh=nEo9jtdnWK4RSL1N6Mtdsig8ikwU/Y0pMwxTSS4HQus=; b=UjpM9cOVh+OMw4wGwzIqixRrHkkr+P7P72ArXcP7hSic7ZoIm9aFToSJQXjehOEIPH jxfQ7H3fVd6iMkRVAu/KEGgFb38sC+LDIhsHj8WarbP7CrOkaflQCQg59u2KdNF4A4XM p/psQq/OesuxtaLSjvd/zzXn/P+vFDIvbff1/uK653x0IDKOciRf6Deu+rn04QIZsP+V 6o2ZstPOxj0VPo5/u35E/jW/lctCUMoZ0vUhqq08SD2hD0O02aBQ6FQp1nIzJZf+eA01 /v6UdG5vhGVSEwboEpkv30r+Grwi47K7mYO3fRX8Zx1SQrcYGfjxY3V8+vatAHK6bNaT m31w== X-Gm-Message-State: APjAAAVYb+enoZKn8cjT4dbygzz7MvCGi7wPWkqMt8L2Pe9GvbPlsOmG /pAKjfKTaPm+SIdE+OPl7j/bQMOvEv2gNKxkgT0= X-Google-Smtp-Source: APXvYqy7mDOJPgnMIQY0y99minLU8TphnZMZXp6SjsA7UIaTo9JH/56Ba3y1YD+IDOaEnLk7dGTncC2DRry6t4/oY2k= X-Received: by 2002:a62:5385:: with SMTP id h127mr49739650pfb.10.1552586946712; Thu, 14 Mar 2019 11:09:06 -0700 (PDT) MIME-Version: 1.0 References: <20190314174416.25935-1-aaptel@suse.com> In-Reply-To: <20190314174416.25935-1-aaptel@suse.com> From: Steve French Date: Thu, 14 Mar 2019 13:08:54 -0500 Message-ID: Subject: Fwd: [RFC][PATCH v1] CIFS: fix POSIX lock leak and invalid ptr deref To: samba-technical , linux-fsdevel Sender: linux-fsdevel-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-fsdevel@vger.kernel.org X-Virus-Scanned: ClamAV using ClamSMTP Adding fsdevel in case any more feedback ... ---------- Forwarded message --------- From: Aurelien Aptel Date: Thu, Mar 14, 2019 at 12:45 PM Subject: [RFC][PATCH v1] CIFS: fix POSIX lock leak and invalid ptr deref To: Cc: Aurelien Aptel , NeilBrown We have a customer reporting crashes in lock_get_status() with many "Leaked POSIX lock" messages preceeding the crash. Leaked POSIX lock on dev=0x0:0x56 ... Leaked POSIX lock on dev=0x0:0x56 ... Leaked POSIX lock on dev=0x0:0x56 ... Leaked POSIX lock on dev=0x0:0x53 ... Leaked POSIX lock on dev=0x0:0x53 ... Leaked POSIX lock on dev=0x0:0x53 ... Leaked POSIX lock on dev=0x0:0x53 ... POSIX: fl_owner=ffff8900e7b79380 fl_flags=0x1 fl_type=0x1 fl_pid=20709 Leaked POSIX lock on dev=0x0:0x4b ino... Leaked locks on dev=0x0:0x4b ino=0xf911400000029: POSIX: fl_owner=ffff89f41c870e00 fl_flags=0x1 fl_type=0x1 fl_pid=19592 stack segment: 0000 [#1] SMP Modules linked in: binfmt_misc msr tcp_diag udp_diag inet_diag unix_diag af_packet_diag netlink_diag rpcsec_gss_krb5 arc4 ecb auth_rpcgss nfsv4 md4 nfs nls_utf8 lockd grace cifs sunrpc ccm dns_resolver fscache af_packet iscsi_ibft iscsi_boot_sysfs vmw_vsock_vmci_transport vsock xfs libcrc32c sb_edac edac_core crct10dif_pclmul crc32_pclmul ghash_clmulni_intel drbg ansi_cprng vmw_balloon aesni_intel aes_x86_64 lrw gf128mul glue_helper ablk_helper cryptd joydev pcspkr vmxnet3 i2c_piix4 vmw_vmci shpchp fjes processor button ac btrfs xor raid6_pq sr_mod cdrom ata_generic sd_mod ata_piix vmwgfx crc32c_intel drm_kms_helper syscopyarea sysfillrect sysimgblt fb_sys_fops ttm serio_raw ahci libahci drm libata vmw_pvscsi sg dm_multipath dm_mod scsi_dh_rdac scsi_dh_emc scsi_dh_alua scsi_mod autofs4 Supported: Yes CPU: 6 PID: 28250 Comm: lsof Not tainted 4.4.156-94.64-default #1 Hardware name: VMware, Inc. VMware Virtual Platform/440BX Desktop Reference Platform, BIOS 6.00 04/05/2016 task: ffff88a345f28740 ti: ffff88c74005c000 task.ti: ffff88c74005c000 RIP: 0010:[] [] lock_get_status+0x9b/0x3b0 RSP: 0018:ffff88c74005fd90 EFLAGS: 00010202 RAX: ffff89bde83e20ae RBX: ffff89e870003d18 RCX: 0000000049534f50 RDX: ffffffff81a3541f RSI: ffffffff81a3544e RDI: ffff89bde83e20ae RBP: 0026252423222120 R08: 0000000020584953 R09: 000000000000ffff R10: 0000000000000000 R11: ffff88c74005fc70 R12: ffff89e5ca7b1340 R13: 00000000000050e5 R14: ffff89e870003d30 R15: ffff89e5ca7b1340 FS: 00007fafd64be800(0000) GS:ffff89f41fd00000(0000) knlGS:0000000000000000 CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033 CR2: 0000000001c80018 CR3: 000000a522048000 CR4: 0000000000360670 DR0: 0000000000000000 DR1: 0000000000000000 DR2: 0000000000000000 DR3: 0000000000000000 DR6: 00000000fffe0ff0 DR7: 0000000000000400 Stack: 0000000000000208 ffffffff81a3d6b6 ffff89e870003d30 ffff89e870003d18 ffff89e5ca7b1340 ffff89f41738d7c0 ffff89e870003d30 ffff89e5ca7b1340 ffffffff8125e08f 0000000000000000 ffff89bc22b67d00 ffff88c74005ff28 Call Trace: [] locks_show+0x2f/0x70 [] seq_read+0x251/0x3a0 [] proc_reg_read+0x3c/0x70 [] __vfs_read+0x26/0x140 [] vfs_read+0x7a/0x120 [] SyS_read+0x42/0xa0 [] entry_SYSCALL_64_fastpath+0x1e/0xb7 When Linux closes a FD (close(), close-on-exec, dup2(), ...) it calls filp_close() which also removes all posix locks. The lock struct is initialized like so in filp_close() and passed down to cifs ... lock.fl_type = F_UNLCK; lock.fl_flags = FL_POSIX | FL_CLOSE; lock.fl_start = 0; lock.fl_end = OFFSET_MAX; ... Note the FL_CLOSE flag, which hints the VFS code that this unlocking is done for closing the fd. filp_close() locks_remove_posix(filp, id); vfs_lock_file(filp, F_SETLK, &lock, NULL); return filp->f_op->lock(filp, cmd, fl) => cifs_lock() rc = cifs_setlk(file, flock, type, wait_flag, posix_lck, lock, unlock, xid); rc = server->ops->mand_unlock_range(cfile, flock, xid); if (flock->fl_flags & FL_POSIX && !rc) rc = locks_lock_file_wait(file, flock) Notice how we don't call locks_lock_file_wait() which does the generic VFS lock/unlock/wait work on the inode if rc != 0. If we are closing the handle, the SMB server is supposed to remove any locks associated with it. Similarly, cifs.ko frees and wakes up any lock and lock waiter when closing the file: cifs_close() cifsFileInfo_put(file->private_data) /* * Delete any outstanding lock records. We'll lose them when the file * is closed anyway. */ down_write(&cifsi->lock_sem); list_for_each_entry_safe(li, tmp, &cifs_file->llist->locks, llist) { list_del(&li->llist); cifs_del_lock_waiters(li); kfree(li); } list_del(&cifs_file->llist->llist); kfree(cifs_file->llist); up_write(&cifsi->lock_sem); So we can safely ignore unlocking failures in cifs_lock() if they happen with the FL_CLOSE flag hint set as both the server and the client take care of it during the actual closing. This is not a proper fix for the unlocking failure but it's safe and it seems to prevent the lock leakages and crashes the customer experiences. Signed-off-by: Aurelien Aptel Signed-off-by: NeilBrown --- fs/cifs/file.c | 14 +++++++++++++- 1 file changed, 13 insertions(+), 1 deletion(-) -- 2.16.4 diff --git a/fs/cifs/file.c b/fs/cifs/file.c index 9b53f33137b3..9cb904afab2f 100644 --- a/fs/cifs/file.c +++ b/fs/cifs/file.c @@ -1645,8 +1645,20 @@ cifs_setlk(struct file *file, struct file_lock *flock, __u32 type, rc = server->ops->mand_unlock_range(cfile, flock, xid); out: - if (flock->fl_flags & FL_POSIX && !rc) + if (flock->fl_flags & FL_POSIX) { + /* + * If this is a request to remove all locks because we + * are closing the file, it doesn't matter if the + * unlocking failed as both cifs.ko and the SMB server + * remove the lock on file close + */ + if (rc) { + cifs_dbg(VFS, "%s failed rc=%d\n", __func__, rc); + if (!(flock->fl_flags & FL_CLOSE)) + return rc; + } rc = locks_lock_file_wait(file, flock); + } return rc; }