Received: by 2002:a25:4158:0:0:0:0:0 with SMTP id o85csp1028168yba; Thu, 4 Apr 2019 02:46:26 -0700 (PDT) X-Google-Smtp-Source: APXvYqxXRb9gRQPpNPKgVt/gHSQhFm8Y/t+I7CVqw+pMo5Y9Pqj/EdHtxZ6ati7ryGIiOeyS4Pui X-Received: by 2002:a62:b40b:: with SMTP id h11mr4854248pfn.133.1554371186333; Thu, 04 Apr 2019 02:46:26 -0700 (PDT) ARC-Seal: i=1; a=rsa-sha256; t=1554371186; cv=none; d=google.com; s=arc-20160816; b=I6mgujiRBWqYQtYw/RZfACgZaGkCicIWptDjqfTpER0hHITXlv4sFUpV+AkClCC7vH z9MOHjko/e593DRJtz9+nqtm9CB0+HwOUk/9i+EP9H0MJvV2JLZp/kNpC84GIroXIF+v eiIOoVZJ4y122Iw2PKMq0s+X2cR+kueqIDFRHQpAlF1ffCQNtrO1iEeIQFsSN1dy5tbQ yOmuTA35uBDwqJADku2w+Hu5QGcMs7+4LM3xcx0ZY9p5jx57cdi15FmvOGRG3r6Q0FJ2 OQyObNs1jO6VI4SAITHkJsvI58HADUyHir4Am9nVQ9R+ZiRMYzWQPbqqwKCsy/T7FWhp cM7w== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=arc-20160816; h=list-id:precedence:sender:content-transfer-encoding:mime-version :user-agent:references:in-reply-to:message-id:date:subject:cc:to :from:dkim-signature; bh=1/RYES8YWUJGOkV2IJBrlZzy2qA9GU0rrFOJq1Gn5LA=; b=NQQJ6OA8Ppe8T7moFzmvpN7kXzsH8Y6vO8vyhxqsjVbS/pQK/1rFF9pSihpXLCEatJ 0xfwrU4TVwa0OA6EIXhORDs60tPLgnFZLEDaocZL4T5PF26pcU/dp5R532+QSyPXRSif iY+KhGUeXP2ODsIzkKR/H72ZueIcAvWqRTCWvYrFLWSpTqRMFF3FfzfdUZ0nAvWcpxcA EmaV9ONAesGihV8zvoN6fqIHSAaiKQUYqnw2BELVwZQ3R4ynQw1QPww0I9G9ezOnbzh4 4VfRGgSjiArucRFR8TfYIHXX+kysjZErZ/F3UMfo3TZ2Ak3POZfOo5L6tiBikYjxHQPe nEKg== ARC-Authentication-Results: i=1; mx.google.com; dkim=pass header.i=@kernel.org header.s=default header.b=qebM25ap; spf=pass (google.com: best guess record for domain of linux-kernel-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org Return-Path: Received: from vger.kernel.org (vger.kernel.org. [209.132.180.67]) by mx.google.com with ESMTP id b125si16451864pfb.242.2019.04.04.02.46.11; Thu, 04 Apr 2019 02:46:26 -0700 (PDT) Received-SPF: pass (google.com: best guess record for domain of linux-kernel-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) client-ip=209.132.180.67; Authentication-Results: mx.google.com; dkim=pass header.i=@kernel.org header.s=default header.b=qebM25ap; spf=pass (google.com: best guess record for domain of linux-kernel-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1730574AbfDDI7t (ORCPT + 99 others); Thu, 4 Apr 2019 04:59:49 -0400 Received: from mail.kernel.org ([198.145.29.99]:36202 "EHLO mail.kernel.org" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1730467AbfDDI7q (ORCPT ); Thu, 4 Apr 2019 04:59:46 -0400 Received: from localhost (83-86-89-107.cable.dynamic.v4.ziggo.nl [83.86.89.107]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by mail.kernel.org (Postfix) with ESMTPSA id 9F62321741; Thu, 4 Apr 2019 08:59:44 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=default; t=1554368385; bh=ItoMWBSE/W+ZHoIOEUyUDqmKwN/FB/k9MxwU3IQVIic=; h=From:To:Cc:Subject:Date:In-Reply-To:References:From; b=qebM25apd6nvPMTlIPQeA3cLUuzLgAHPWW3v3+mCNveQQcfk5t552S2tVKB6sSUUT EYf5cZdoxtFl8SN2WIweu8NMci2X5RRdp7ChkQO89hnSTM/veIBBLjHfUD8UhNdM2x 9C/XVs44M9Hj4tFij8/haVJ1Ghc8dYt6KPEmoI20= From: Greg Kroah-Hartman To: linux-kernel@vger.kernel.org Cc: Greg Kroah-Hartman , stable@vger.kernel.org, Aurelien Aptel , NeilBrown , Steve French , Pavel Shilovsky , Sasha Levin Subject: [PATCH 4.19 005/187] CIFS: fix POSIX lock leak and invalid ptr deref Date: Thu, 4 Apr 2019 10:45:42 +0200 Message-Id: <20190404084603.352003772@linuxfoundation.org> X-Mailer: git-send-email 2.21.0 In-Reply-To: <20190404084603.119654039@linuxfoundation.org> References: <20190404084603.119654039@linuxfoundation.org> User-Agent: quilt/0.65 X-stable: review X-Patchwork-Hint: ignore MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Sender: linux-kernel-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-kernel@vger.kernel.org 4.19-stable review patch. If anyone has any objections, please let me know. ------------------ [ Upstream commit bc31d0cdcfbadb6258b45db97e93b1c83822ba33 ] 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 Signed-off-by: Steve French Acked-by: Pavel Shilovsky Signed-off-by: Sasha Levin --- fs/cifs/file.c | 14 +++++++++++++- 1 file changed, 13 insertions(+), 1 deletion(-) diff --git a/fs/cifs/file.c b/fs/cifs/file.c index 08761a6a039d..d847132ab027 100644 --- a/fs/cifs/file.c +++ b/fs/cifs/file.c @@ -1631,8 +1631,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; } -- 2.19.1