From patchwork Mon Oct 30 20:19:53 2023 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Paulo Alcantara X-Patchwork-Id: 13440854 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by smtp.lore.kernel.org (Postfix) with ESMTP id C0FAAC4332F for ; Mon, 30 Oct 2023 20:20:10 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S229517AbjJ3UUL (ORCPT ); Mon, 30 Oct 2023 16:20:11 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:39516 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S229456AbjJ3UUK (ORCPT ); Mon, 30 Oct 2023 16:20:10 -0400 Received: from mx.manguebit.com (mx.manguebit.com [167.235.159.17]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id 4042BDF for ; Mon, 30 Oct 2023 13:20:07 -0700 (PDT) From: Paulo Alcantara DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=manguebit.com; s=dkim; t=1698697205; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version: content-transfer-encoding:content-transfer-encoding; bh=H+IHXSCrkRjIjf8P6Ep3tsNeuTeJRdFAJe3l88hY7QI=; b=m26E3y6i03yitBTAOAcmZOm0f3iuEbi//psG2UEVxDMKfv/MMveOk4qn9ocaqA77hNQVzx OqWEtTia1jHzT1L2YWwru/mQqEZ5PCoEduxu7FCJ9pz4BxHHv/UfGq4yanm4EXze9Bscnp tqMv7oRIIgNlT88huF0InROtrmGnR+QGyOsP5v6epz5fBeQlASfTxyZVz0A5s92mD1z+H6 RYSwCTqEr8xy28zqMrz+/JrxqnnqCQTZLd/jHBmaurYY4HtkLcAOtHvOYTBbiZAaxqPJXg VBZZqD+4toGsVhwII/stqAmGFXKM4O6z06HCilS4nj3gexKGBBjtsD42iWRgzg== ARC-Seal: i=1; s=dkim; d=manguebit.com; t=1698697205; a=rsa-sha256; cv=none; b=EFY6OluLs2Zen7yPnAwHaEIF+lckmjNFmsRA+XlS7YzqfK5kf59dx8QiQ5fy1t/m4WRr0v rBmguyssmVWIgzb9hfOv3RprSWEHpdmKYxF7FatoZVlsJjra5TgWE+EwdKXVo/+ljKDjJy d8x44oFb+K61U5Qo0cTS6N6o2RQYa81hvLnkk2azewKn2ZT4V6Wpw7QL3uu1zjcGEVAvPk hdrehlxpXmzUTHI+70cHqrOqJI1KsW2GXIzXeo5+GBJv3c0z1R/hQNwKfETXj2ZRYBaYeR ErtdA2gzvMCbkerNvf5I3ZEHhwfk3r+FOVph0ACsd2Q3DqRJ5EOPnE+6IV4byQ== ARC-Authentication-Results: i=1; ORIGINATING; auth=pass smtp.auth=pc@manguebit.com smtp.mailfrom=pc@manguebit.com ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=manguebit.com; s=dkim; t=1698697205; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version: content-transfer-encoding:content-transfer-encoding; bh=H+IHXSCrkRjIjf8P6Ep3tsNeuTeJRdFAJe3l88hY7QI=; b=WlFAycKuzxC1T7BxSzh9lYo6GuVrtub1xVpn457uKh54tteJorHeFt0VYpf0LwT4SrF2kv oEXY0PCM1yGLx64KVV4g17rSCNn48vzPIAFBgRWLnWmaYeDWTIc/mIx4AxNSQ1kxPC1I7L mu8noCys26qjUzh/8TGI0geK6K+BWiCkgzmlzTXg+GdoP8JeTVwLGbniE0qJq8kvb2FZqQ UlqPZQ7uo58pTSizQL6uTa0cmUZtAX+QPi5LVj/ng/U9ySd5/ejay7K5a9nb2W7L8qPRUt A2q0/NCbe1n//Dsu6OAkZo/XRNl125pMLHAYqMVagyhYoZCLRIY4PfQk+ffZbQ== To: smfrench@gmail.com Cc: linux-cifs@vger.kernel.org, Paulo Alcantara Subject: [PATCH 1/4] smb: client: remove extra @chan_count check in __cifs_put_smb_ses() Date: Mon, 30 Oct 2023 17:19:53 -0300 Message-ID: <20231030201956.2660-1-pc@manguebit.com> MIME-Version: 1.0 Precedence: bulk List-ID: X-Mailing-List: linux-cifs@vger.kernel.org If @ses->chan_count <= 1, then for-loop body will not be executed so no need to check it twice. Signed-off-by: Paulo Alcantara (SUSE) --- fs/smb/client/connect.c | 23 +++++++++-------------- 1 file changed, 9 insertions(+), 14 deletions(-) diff --git a/fs/smb/client/connect.c b/fs/smb/client/connect.c index 7b923e36501b..a017ee552256 100644 --- a/fs/smb/client/connect.c +++ b/fs/smb/client/connect.c @@ -1969,9 +1969,10 @@ cifs_find_smb_ses(struct TCP_Server_Info *server, struct smb3_fs_context *ctx) void __cifs_put_smb_ses(struct cifs_ses *ses) { - unsigned int rc, xid; - unsigned int chan_count; struct TCP_Server_Info *server = ses->server; + unsigned int xid; + size_t i; + int rc; spin_lock(&ses->ses_lock); if (ses->ses_status == SES_EXITING) { @@ -2017,20 +2018,14 @@ void __cifs_put_smb_ses(struct cifs_ses *ses) list_del_init(&ses->smb_ses_list); spin_unlock(&cifs_tcp_ses_lock); - chan_count = ses->chan_count; - /* close any extra channels */ - if (chan_count > 1) { - int i; - - for (i = 1; i < chan_count; i++) { - if (ses->chans[i].iface) { - kref_put(&ses->chans[i].iface->refcount, release_iface); - ses->chans[i].iface = NULL; - } - cifs_put_tcp_session(ses->chans[i].server, 0); - ses->chans[i].server = NULL; + for (i = 1; i < ses->chan_count; i++) { + if (ses->chans[i].iface) { + kref_put(&ses->chans[i].iface->refcount, release_iface); + ses->chans[i].iface = NULL; } + cifs_put_tcp_session(ses->chans[i].server, 0); + ses->chans[i].server = NULL; } sesInfoFree(ses); From patchwork Mon Oct 30 20:19:54 2023 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Paulo Alcantara X-Patchwork-Id: 13440855 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by smtp.lore.kernel.org (Postfix) with ESMTP id 2F645C4167D for ; Mon, 30 Oct 2023 20:20:11 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S229456AbjJ3UUM (ORCPT ); Mon, 30 Oct 2023 16:20:12 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:39518 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S229485AbjJ3UUL (ORCPT ); Mon, 30 Oct 2023 16:20:11 -0400 Received: from mx.manguebit.com (mx.manguebit.com [IPv6:2a01:4f8:1c1e:a2ae::2]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id 15C91F7 for ; Mon, 30 Oct 2023 13:20:09 -0700 (PDT) From: Paulo Alcantara DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=manguebit.com; s=dkim; t=1698697207; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=H6wAReVm2ABWu6tWfK+vmOedUVKy+h4P9/PxypfJcgw=; b=Tiuut6djOfWkGfg7iWwmImoOGw3S0gqwSivUvX03AAi0P/wIlRnBh2Zjw+X1bh4GzkIr4A d8mhd8uTMeqN/R5QRRP+SablSpywIg/DkACre55sd7HQTPm5KphUIL1JfUdNyIa7KX7bJK 6n09FFYfQKvWYx42uccA+3cdx2+Qd6MMhFMZZYomCPfaq2DupO/2Pcpb/8432I9OoQRSZl bbHQu71HDG3wXQQDJkHzpiHuFqgi1jfwNOwk6HycWJRYdRhSfuj4/ngKfzmKFTn4HJnQcG a68L87BnH5c14KmjteDVTE2Mex0aKoFuqwHMdz4tYhyyWzDs9BLOGoLBaH2waQ== ARC-Seal: i=1; s=dkim; d=manguebit.com; t=1698697207; a=rsa-sha256; cv=none; b=K9GMapi7xW4dHuSMK7KtZcuDVjLhr5dPE7ZBk6sM+4Uv7Hqy+x01gogmCXzQibT/LixoLs jU7oS9N3NfOjSg5QPlCiwRSEIqpE8SfjMCR8X41WcTujAKv46rV0qjjOmi4vq8e6IKz8e1 jthrtaQjxhSKezoWNeggPkLN3PDkE9eCBG9j4kptab9kV+D5og2qzidZ0fCm9mBJN1Aks9 g+QFv176TOJk5HSV58ij2IuFjo62VD1ryHvTXA8FvYgu1lisje+3NXhUhQHBECxnvTmzey smxDDUmr13xdEZOVAbxP7egfhmYMAJM41o38+uuftnTmd2SyF02rih60ymkIpw== ARC-Authentication-Results: i=1; ORIGINATING; auth=pass smtp.auth=pc@manguebit.com smtp.mailfrom=pc@manguebit.com ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=manguebit.com; s=dkim; t=1698697207; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=H6wAReVm2ABWu6tWfK+vmOedUVKy+h4P9/PxypfJcgw=; b=BXqcuDpqge1a2eUW8oMuY5AA2WkAjg+w7rSQVY+E4JYamML83Jrxah+IRxelNz+auoWyfc WokqZooVzYfD21ducQBkj2kOVIag6gfgeBr6PRARW9JzWEs1fbpDHMwxpPfJlPVJjglrkS kS8yv3MUgZvgmDD/pctYs/0Xgm5rSeImxdzQYDYdmYN7+RGCNaaRCEg6rsTPV9ctx38tIn Bu1QeXd/GAMyy1KAxSjUyja5ei22Tgn07kbLjbmA/8aydI0D/cTSQGbmY6p49D96hTR/eZ iMU98cQpDNoxW7z8FJ4AEduamQIfM6Xp0XdobaxRMunDfqXka/fqjJRy+jjHBQ== To: smfrench@gmail.com Cc: linux-cifs@vger.kernel.org, Paulo Alcantara , Frank Sorenson Subject: [PATCH 2/4] smb: client: fix use-after-free bug in cifs_debug_data_proc_show() Date: Mon, 30 Oct 2023 17:19:54 -0300 Message-ID: <20231030201956.2660-2-pc@manguebit.com> In-Reply-To: <20231030201956.2660-1-pc@manguebit.com> References: <20231030201956.2660-1-pc@manguebit.com> MIME-Version: 1.0 Precedence: bulk List-ID: X-Mailing-List: linux-cifs@vger.kernel.org Skip SMB sessions that are being teared down (e.g. @ses->ses_status == SES_EXITING) in cifs_debug_data_proc_show() to avoid use-after-free in @ses. This fixes the following GPF when reading from /proc/fs/cifs/DebugData while mounting and umounting [ 816.251274] general protection fault, probably for non-canonical address 0x6b6b6b6b6b6b6d81: 0000 [#1] PREEMPT SMP NOPTI ... [ 816.260138] Call Trace: [ 816.260329] [ 816.260499] ? die_addr+0x36/0x90 [ 816.260762] ? exc_general_protection+0x1b3/0x410 [ 816.261126] ? asm_exc_general_protection+0x26/0x30 [ 816.261502] ? cifs_debug_tcon+0xbd/0x240 [cifs] [ 816.261878] ? cifs_debug_tcon+0xab/0x240 [cifs] [ 816.262249] cifs_debug_data_proc_show+0x516/0xdb0 [cifs] [ 816.262689] ? seq_read_iter+0x379/0x470 [ 816.262995] seq_read_iter+0x118/0x470 [ 816.263291] proc_reg_read_iter+0x53/0x90 [ 816.263596] ? srso_alias_return_thunk+0x5/0x7f [ 816.263945] vfs_read+0x201/0x350 [ 816.264211] ksys_read+0x75/0x100 [ 816.264472] do_syscall_64+0x3f/0x90 [ 816.264750] entry_SYSCALL_64_after_hwframe+0x6e/0xd8 [ 816.265135] RIP: 0033:0x7fd5e669d381 Cc: Frank Sorenson Signed-off-by: Paulo Alcantara (SUSE) --- fs/smb/client/cifs_debug.c | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/fs/smb/client/cifs_debug.c b/fs/smb/client/cifs_debug.c index 76922fcc4bc6..9a0ccd87468e 100644 --- a/fs/smb/client/cifs_debug.c +++ b/fs/smb/client/cifs_debug.c @@ -452,6 +452,11 @@ static int cifs_debug_data_proc_show(struct seq_file *m, void *v) seq_printf(m, "\n\n\tSessions: "); i = 0; list_for_each_entry(ses, &server->smb_ses_list, smb_ses_list) { + spin_lock(&ses->ses_lock); + if (ses->ses_status == SES_EXITING) { + spin_unlock(&ses->ses_lock); + continue; + } i++; if ((ses->serverDomain == NULL) || (ses->serverOS == NULL) || @@ -472,6 +477,7 @@ static int cifs_debug_data_proc_show(struct seq_file *m, void *v) ses->ses_count, ses->serverOS, ses->serverNOS, ses->capabilities, ses->ses_status); } + spin_unlock(&ses->ses_lock); seq_printf(m, "\n\tSecurity type: %s ", get_security_type_str(server->ops->select_sectype(server, ses->sectype))); From patchwork Mon Oct 30 20:19:55 2023 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Paulo Alcantara X-Patchwork-Id: 13440856 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by smtp.lore.kernel.org (Postfix) with ESMTP id 29616C4167B for ; Mon, 30 Oct 2023 20:20:13 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S229557AbjJ3UUN (ORCPT ); Mon, 30 Oct 2023 16:20:13 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:39540 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S229485AbjJ3UUM (ORCPT ); Mon, 30 Oct 2023 16:20:12 -0400 Received: from mx.manguebit.com (mx.manguebit.com [167.235.159.17]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id CC747F7 for ; Mon, 30 Oct 2023 13:20:10 -0700 (PDT) From: Paulo Alcantara DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=manguebit.com; s=dkim; t=1698697209; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=KWkXBtgQshXyjb+A1gCmJORlvEBsTyCPFTAr86WpZ2I=; b=RsQRFJc+q9oZuqiBfnAYPZCewxKPWb4mzm0CP79rQv2zO6KyK12sa+O4NamIWUQn/3ijMm kICbOD5oC6fQyzzDGGrgsyUNxfltrwjZzB1oDhp8Vzr2Bd0nb4rk5INhrbJAVbIG+CXyLT fix5zS3nW1GLyRjZ6TsKPUhiN5C08UIdOu9m9S3y6V5U8UqEazEMobfJDCGzN/rV3gvqNv V5qpZgKM1xmq8bf491yjdvxXejh/r1lmocVZbSNCOPiTaL1u0SngoEuC3xxPlRKQwnZDHf YxCP1fZoKJKhlPLqeyY24EOjN+Iuo6y4n3jnUD1OqFbMmnsoYJpcC/siGGLlnw== ARC-Seal: i=1; s=dkim; d=manguebit.com; t=1698697209; a=rsa-sha256; cv=none; b=J0YADWLuoDTZ29NqD/JUzhB91W9lT3KFNVEjx7mVBbTQbuJtRPqBp+2CNZRI7dq9phj37b rNTa8KGA7//h7JI/9++wNazpdT08JxQW/WyXY+NZa2vb4OcKOCrG8UNmj/vltAX3isb9+b gysPRGvZ3P+MhuYzRmAwpk791HdD1dC3iDrXNE11iqVGNGDOhwaEphklnbhaACqZUfYodK DpsyEsa6piiEu0kzsentYcTuI2U2doySn36/dpWOqZP8iRHOMRlf/r4hLfNMIj0Pu+mN00 pJYNQVNdfqhd6cLBuzIOPGP0CjhLh8PN+nWv58hhuIj3AeXZ5TzZnkDlneZ+Hg== ARC-Authentication-Results: i=1; ORIGINATING; auth=pass smtp.auth=pc@manguebit.com smtp.mailfrom=pc@manguebit.com ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=manguebit.com; s=dkim; t=1698697209; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=KWkXBtgQshXyjb+A1gCmJORlvEBsTyCPFTAr86WpZ2I=; b=jIi2uRhXWsC0CchYvHy+vuN/niLciSxZ4DERhX5biZ+Xg+O99L1pCt6zuhSlHnGYj7ol46 YOTihQyYA/fYvzeDCycuk6Cty1UH+Tw0w/Nl5CkaqMx8Lk6HKMcQemnHauQ/4oWs+xQXBc Pegd8ij09hLvnjYeiEclM9pileoezGX/l5yFYcubCDIasb5gDEHu4by+DI8vvVfwoL5Tv3 4Ebjo9jUuEsXwCVD1SpIxCZT4OGd8aQaU2qY+GKKrEvkBNLhWjqGk6scs3Y8Zz/SFokyQv VzuciGjjYP719D3qTBQLOI5TNAI4F0bqoIdL6HlAd4ThLzQ3AzNyTaNWG/KRqQ== To: smfrench@gmail.com Cc: linux-cifs@vger.kernel.org, Paulo Alcantara , Frank Sorenson Subject: [PATCH 3/4] smb: client: fix potential deadlock when releasing mids Date: Mon, 30 Oct 2023 17:19:55 -0300 Message-ID: <20231030201956.2660-3-pc@manguebit.com> In-Reply-To: <20231030201956.2660-1-pc@manguebit.com> References: <20231030201956.2660-1-pc@manguebit.com> MIME-Version: 1.0 Precedence: bulk List-ID: X-Mailing-List: linux-cifs@vger.kernel.org All release_mid() callers seem to hold a reference of @mid so there is no need to call kref_put(&mid->refcount, __release_mid) under @server->mid_lock spinlock. If they don't, then an use-after-free bug would have occurred anyway. By getting rid of such spinlock also fixes a potential deadlock as shown below CPU 0 CPU 1 ------------------------------------------------------------------ cifs_demultiplex_thread() cifs_debug_data_proc_show() release_mid() spin_lock(&server->mid_lock); spin_lock(&cifs_tcp_ses_lock) spin_lock(&server->mid_lock) __release_mid() smb2_find_smb_tcon() spin_lock(&cifs_tcp_ses_lock) *deadlock* Cc: Frank Sorenson Signed-off-by: Paulo Alcantara (SUSE) --- fs/smb/client/cifsproto.h | 7 ++++++- fs/smb/client/smb2misc.c | 2 +- fs/smb/client/transport.c | 11 +---------- 3 files changed, 8 insertions(+), 12 deletions(-) diff --git a/fs/smb/client/cifsproto.h b/fs/smb/client/cifsproto.h index 0c37eefa18a5..890ceddae07e 100644 --- a/fs/smb/client/cifsproto.h +++ b/fs/smb/client/cifsproto.h @@ -81,7 +81,7 @@ extern char *cifs_build_path_to_root(struct smb3_fs_context *ctx, extern char *build_wildcard_path_from_dentry(struct dentry *direntry); char *cifs_build_devname(char *nodename, const char *prepath); extern void delete_mid(struct mid_q_entry *mid); -extern void release_mid(struct mid_q_entry *mid); +void __release_mid(struct kref *refcount); extern void cifs_wake_up_task(struct mid_q_entry *mid); extern int cifs_handle_standard(struct TCP_Server_Info *server, struct mid_q_entry *mid); @@ -740,4 +740,9 @@ static inline bool dfs_src_pathname_equal(const char *s1, const char *s2) return true; } +static inline void release_mid(struct mid_q_entry *mid) +{ + kref_put(&mid->refcount, __release_mid); +} + #endif /* _CIFSPROTO_H */ diff --git a/fs/smb/client/smb2misc.c b/fs/smb/client/smb2misc.c index 25f7cd6f23d6..32dfa0f7a78c 100644 --- a/fs/smb/client/smb2misc.c +++ b/fs/smb/client/smb2misc.c @@ -787,7 +787,7 @@ __smb2_handle_cancelled_cmd(struct cifs_tcon *tcon, __u16 cmd, __u64 mid, { struct close_cancelled_open *cancelled; - cancelled = kzalloc(sizeof(*cancelled), GFP_ATOMIC); + cancelled = kzalloc(sizeof(*cancelled), GFP_KERNEL); if (!cancelled) return -ENOMEM; diff --git a/fs/smb/client/transport.c b/fs/smb/client/transport.c index 14710afdc2a3..d553b7a54621 100644 --- a/fs/smb/client/transport.c +++ b/fs/smb/client/transport.c @@ -76,7 +76,7 @@ alloc_mid(const struct smb_hdr *smb_buffer, struct TCP_Server_Info *server) return temp; } -static void __release_mid(struct kref *refcount) +void __release_mid(struct kref *refcount) { struct mid_q_entry *midEntry = container_of(refcount, struct mid_q_entry, refcount); @@ -156,15 +156,6 @@ static void __release_mid(struct kref *refcount) mempool_free(midEntry, cifs_mid_poolp); } -void release_mid(struct mid_q_entry *mid) -{ - struct TCP_Server_Info *server = mid->server; - - spin_lock(&server->mid_lock); - kref_put(&mid->refcount, __release_mid); - spin_unlock(&server->mid_lock); -} - void delete_mid(struct mid_q_entry *mid) { From patchwork Mon Oct 30 20:19:56 2023 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Paulo Alcantara X-Patchwork-Id: 13440857 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by smtp.lore.kernel.org (Postfix) with ESMTP id 9B208C4332F for ; Mon, 30 Oct 2023 20:20:16 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S229569AbjJ3UUQ (ORCPT ); Mon, 30 Oct 2023 16:20:16 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:39550 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S229485AbjJ3UUP (ORCPT ); Mon, 30 Oct 2023 16:20:15 -0400 Received: from mx.manguebit.com (mx.manguebit.com [167.235.159.17]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id 7D950DF for ; Mon, 30 Oct 2023 13:20:12 -0700 (PDT) From: Paulo Alcantara DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=manguebit.com; s=dkim; t=1698697211; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=vWN/cZPo1MpY75LHtWVz+rQTpsWFBWw6HEjK8xnBPXU=; b=VUMtRufpeY/VJshEYqkH7iCpRyXQoe3mnSb/mHUWBI0qYn4WnwtSCIrGVh3GGjlkanCul+ n/PFrx+ORYlvOeMrewZxGvrbNnqSBDafPUFm2Fq/RS5oGFWRJYegrFjYj51YjAi0+H0tfX FfLlTXSyrx5U0U8BVmCJqyfH/mSx0ws9iHWSCYrMerJPumAr58RPkgOHIhkucX50xmBObU Mclfc6ePREdF02Pq909aeogZW+mUoJQLWiUcHuWPE/ZYQNXtuOj+5xufVQGiauavo/iR5T Vp8BGh8GXihuiO0W7sW7Iw8lrBu4Vk9PUxnXz1sv9fLPwUJ+QGGm8tJa9wVy6w== ARC-Seal: i=1; s=dkim; d=manguebit.com; t=1698697211; a=rsa-sha256; cv=none; b=EfAD6Mv/ajStG6YxOofP1QLn7zAK3MQMSNZezuuzPWCHOctxh2+l5tv4zGbZ0qJBjA3qmZ Rf6BFYYg3c4idUULdzcKOlQFHI67JZR3eeyoh1zizHU1s4ReA0E9o/BQg6nDrMMBPswpq/ NIqPutMKidDZQ0MDr+wFsbVVnsgOr1ji5IOCwxpMqrhBWQF7cJhTEPkSDIpoCBBHyB3vDY mRTYGkcJg+w0tgePG44xFWNo0Da0OkzHWZeFTu35ogkcgzxzTcrNXgLhTpzp6r3IqrLvo9 nZ9qi/MM1FaeGqMZe4m3dRK+DgWOqreaImyIz+SCConSizc2c4UBy7FX5t9W3Q== ARC-Authentication-Results: i=1; ORIGINATING; auth=pass smtp.auth=pc@manguebit.com smtp.mailfrom=pc@manguebit.com ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=manguebit.com; s=dkim; t=1698697211; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=vWN/cZPo1MpY75LHtWVz+rQTpsWFBWw6HEjK8xnBPXU=; b=TpBqHObcEunH6012bt15D6svXFI0R87qyZZudIQe1dfWbqvF536wQa8j3XwECrPmPZJoDC XooAqIEnc3Rd/V92yroNiQWyTcPyp+H0PAEP8T2J24s82Iu9u/W2gkp3T7ny543kvZyPsX c0D1yK6M2b7tzGvcy0jz0eT29fz5L+TKwXkE2d/NF/HBS1B0boUdvh46B8xZ3ySwTa9GC3 LFINwalRY7x+eG5bxVbuWVYuFpTsOKH/MntfhNryAeQ+NdKHyp9PCNVPjWGVHwQsjTfb5J 8xGjE2Y8DAFZxOMaZl4pEOjYFxvRlBSrLloUF6IIgV1afIF6vLsv/zLqHR3sWQ== To: smfrench@gmail.com Cc: linux-cifs@vger.kernel.org, Paulo Alcantara Subject: [PATCH 4/4] smb: client: fix use-after-free in smb2_query_info_compound() Date: Mon, 30 Oct 2023 17:19:56 -0300 Message-ID: <20231030201956.2660-4-pc@manguebit.com> In-Reply-To: <20231030201956.2660-1-pc@manguebit.com> References: <20231030201956.2660-1-pc@manguebit.com> MIME-Version: 1.0 Precedence: bulk List-ID: X-Mailing-List: linux-cifs@vger.kernel.org The following UAF was triggered when running fstests generic/072 with KASAN enabled against Windows Server 2022 and mount options 'multichannel,max_channels=2,vers=3.1.1,mfsymlinks,noperm' BUG: KASAN: slab-use-after-free in smb2_query_info_compound+0x423/0x6d0 [cifs] Read of size 8 at addr ffff888014941048 by task xfs_io/27534 CPU: 0 PID: 27534 Comm: xfs_io Not tainted 6.6.0-rc7 #1 Hardware name: QEMU Standard PC (Q35 + ICH9, 2009), BIOS rel-1.16.2-3-gd478f380-rebuilt.opensuse.org 04/01/2014 Call Trace: dump_stack_lvl+0x4a/0x80 print_report+0xcf/0x650 ? srso_alias_return_thunk+0x5/0x7f ? srso_alias_return_thunk+0x5/0x7f ? __phys_addr+0x46/0x90 kasan_report+0xda/0x110 ? smb2_query_info_compound+0x423/0x6d0 [cifs] ? smb2_query_info_compound+0x423/0x6d0 [cifs] smb2_query_info_compound+0x423/0x6d0 [cifs] ? __pfx_smb2_query_info_compound+0x10/0x10 [cifs] ? srso_alias_return_thunk+0x5/0x7f ? __stack_depot_save+0x39/0x480 ? kasan_save_stack+0x33/0x60 ? kasan_set_track+0x25/0x30 ? ____kasan_slab_free+0x126/0x170 smb2_queryfs+0xc2/0x2c0 [cifs] ? __pfx_smb2_queryfs+0x10/0x10 [cifs] ? __pfx___lock_acquire+0x10/0x10 smb311_queryfs+0x210/0x220 [cifs] ? __pfx_smb311_queryfs+0x10/0x10 [cifs] ? srso_alias_return_thunk+0x5/0x7f ? __lock_acquire+0x480/0x26c0 ? lock_release+0x1ed/0x640 ? srso_alias_return_thunk+0x5/0x7f ? do_raw_spin_unlock+0x9b/0x100 cifs_statfs+0x18c/0x4b0 [cifs] statfs_by_dentry+0x9b/0xf0 fd_statfs+0x4e/0xb0 __do_sys_fstatfs+0x7f/0xe0 ? __pfx___do_sys_fstatfs+0x10/0x10 ? srso_alias_return_thunk+0x5/0x7f ? lockdep_hardirqs_on_prepare+0x136/0x200 ? srso_alias_return_thunk+0x5/0x7f do_syscall_64+0x3f/0x90 entry_SYSCALL_64_after_hwframe+0x6e/0xd8 Allocated by task 27534: kasan_save_stack+0x33/0x60 kasan_set_track+0x25/0x30 __kasan_kmalloc+0x8f/0xa0 open_cached_dir+0x71b/0x1240 [cifs] smb2_query_info_compound+0x5c3/0x6d0 [cifs] smb2_queryfs+0xc2/0x2c0 [cifs] smb311_queryfs+0x210/0x220 [cifs] cifs_statfs+0x18c/0x4b0 [cifs] statfs_by_dentry+0x9b/0xf0 fd_statfs+0x4e/0xb0 __do_sys_fstatfs+0x7f/0xe0 do_syscall_64+0x3f/0x90 entry_SYSCALL_64_after_hwframe+0x6e/0xd8 Freed by task 27534: kasan_save_stack+0x33/0x60 kasan_set_track+0x25/0x30 kasan_save_free_info+0x2b/0x50 ____kasan_slab_free+0x126/0x170 slab_free_freelist_hook+0xd0/0x1e0 __kmem_cache_free+0x9d/0x1b0 open_cached_dir+0xff5/0x1240 [cifs] smb2_query_info_compound+0x5c3/0x6d0 [cifs] smb2_queryfs+0xc2/0x2c0 [cifs] This is a race between open_cached_dir() and cached_dir_lease_break() where the cache entry for the open directory handle receives a lease break while creating it. And before returning from open_cached_dir(), we put the last reference of the new @cfid because of !@cfid->has_lease. Besides the UAF, while running xfstests a lot of missed lease breaks have been noticed in tests that run several concurrent statfs(2) calls on those cached fids CIFS: VFS: \\w22-root1.gandalf.test No task to wake, unknown frame... CIFS: VFS: \\w22-root1.gandalf.test Cmd: 18 Err: 0x0 Flags: 0x1... CIFS: VFS: \\w22-root1.gandalf.test smb buf 00000000715bfe83 len 108 CIFS: VFS: Dump pending requests: CIFS: VFS: \\w22-root1.gandalf.test No task to wake, unknown frame... CIFS: VFS: \\w22-root1.gandalf.test Cmd: 18 Err: 0x0 Flags: 0x1... CIFS: VFS: \\w22-root1.gandalf.test smb buf 000000005aa7316e len 108 ... To fix both, in open_cached_dir() ensure that @cfid->has_lease is set right before sending out compounded request so that any potential lease break will be get processed by demultiplex thread while we're still caching @cfid. And, if open failed for some reason, re-check @cfid->has_lease to decide whether or not put lease reference. Signed-off-by: Paulo Alcantara (SUSE) --- fs/smb/client/cached_dir.c | 84 ++++++++++++++++++++++---------------- 1 file changed, 49 insertions(+), 35 deletions(-) diff --git a/fs/smb/client/cached_dir.c b/fs/smb/client/cached_dir.c index fe1bf5b6e0cb..59f6b8e32cc9 100644 --- a/fs/smb/client/cached_dir.c +++ b/fs/smb/client/cached_dir.c @@ -32,7 +32,7 @@ static struct cached_fid *find_or_create_cached_dir(struct cached_fids *cfids, * fully cached or it may be in the process of * being deleted due to a lease break. */ - if (!cfid->has_lease) { + if (!cfid->time || !cfid->has_lease) { spin_unlock(&cfids->cfid_list_lock); return NULL; } @@ -193,10 +193,20 @@ int open_cached_dir(unsigned int xid, struct cifs_tcon *tcon, npath = path_no_prefix(cifs_sb, path); if (IS_ERR(npath)) { rc = PTR_ERR(npath); - kfree(utf16_path); - return rc; + goto out; } + if (!npath[0]) { + dentry = dget(cifs_sb->root); + } else { + dentry = path_to_dentry(cifs_sb, npath); + if (IS_ERR(dentry)) { + rc = -ENOENT; + goto out; + } + } + cfid->dentry = dentry; + /* * We do not hold the lock for the open because in case * SMB2_open needs to reconnect. @@ -249,6 +259,15 @@ int open_cached_dir(unsigned int xid, struct cifs_tcon *tcon, smb2_set_related(&rqst[1]); + /* + * Set @cfid->has_lease to true before sending out compounded request so + * its lease reference can be put in cached_dir_lease_break() due to a + * potential lease break right after the request is sent or while @cfid + * is still being cached. Concurrent processes won't be to use it yet + * due to @cfid->time being zero. + */ + cfid->has_lease = true; + rc = compound_send_recv(xid, ses, server, flags, 2, rqst, resp_buftype, rsp_iov); @@ -263,6 +282,8 @@ int open_cached_dir(unsigned int xid, struct cifs_tcon *tcon, cfid->tcon = tcon; cfid->is_open = true; + spin_lock(&cfids->cfid_list_lock); + o_rsp = (struct smb2_create_rsp *)rsp_iov[0].iov_base; oparms.fid->persistent_fid = o_rsp->PersistentFileId; oparms.fid->volatile_fid = o_rsp->VolatileFileId; @@ -270,18 +291,25 @@ int open_cached_dir(unsigned int xid, struct cifs_tcon *tcon, oparms.fid->mid = le64_to_cpu(o_rsp->hdr.MessageId); #endif /* CIFS_DEBUG2 */ - if (o_rsp->OplockLevel != SMB2_OPLOCK_LEVEL_LEASE) + rc = -EINVAL; + if (o_rsp->OplockLevel != SMB2_OPLOCK_LEVEL_LEASE) { + spin_unlock(&cfids->cfid_list_lock); goto oshr_free; + } smb2_parse_contexts(server, o_rsp, &oparms.fid->epoch, oparms.fid->lease_key, &oplock, NULL, NULL); - if (!(oplock & SMB2_LEASE_READ_CACHING_HE)) + if (!(oplock & SMB2_LEASE_READ_CACHING_HE)) { + spin_unlock(&cfids->cfid_list_lock); goto oshr_free; + } qi_rsp = (struct smb2_query_info_rsp *)rsp_iov[1].iov_base; - if (le32_to_cpu(qi_rsp->OutputBufferLength) < sizeof(struct smb2_file_all_info)) + if (le32_to_cpu(qi_rsp->OutputBufferLength) < sizeof(struct smb2_file_all_info)) { + spin_unlock(&cfids->cfid_list_lock); goto oshr_free; + } if (!smb2_validate_and_copy_iov( le16_to_cpu(qi_rsp->OutputBufferOffset), sizeof(struct smb2_file_all_info), @@ -289,37 +317,24 @@ int open_cached_dir(unsigned int xid, struct cifs_tcon *tcon, (char *)&cfid->file_all_info)) cfid->file_all_info_is_valid = true; - if (!npath[0]) - dentry = dget(cifs_sb->root); - else { - dentry = path_to_dentry(cifs_sb, npath); - if (IS_ERR(dentry)) { - rc = -ENOENT; - goto oshr_free; - } - } - spin_lock(&cfids->cfid_list_lock); - cfid->dentry = dentry; cfid->time = jiffies; - cfid->has_lease = true; spin_unlock(&cfids->cfid_list_lock); + /* At this point the directory handle is fully cached */ + rc = 0; oshr_free: - kfree(utf16_path); SMB2_open_free(&rqst[0]); SMB2_query_info_free(&rqst[1]); free_rsp_buf(resp_buftype[0], rsp_iov[0].iov_base); free_rsp_buf(resp_buftype[1], rsp_iov[1].iov_base); - spin_lock(&cfids->cfid_list_lock); - if (!cfid->has_lease) { - if (rc) { - if (cfid->on_list) { - list_del(&cfid->entry); - cfid->on_list = false; - cfids->num_entries--; - } - rc = -ENOENT; - } else { + if (rc) { + spin_lock(&cfids->cfid_list_lock); + if (cfid->on_list) { + list_del(&cfid->entry); + cfid->on_list = false; + cfids->num_entries--; + } + if (cfid->has_lease) { /* * We are guaranteed to have two references at this * point. One for the caller and one for a potential @@ -327,25 +342,24 @@ int open_cached_dir(unsigned int xid, struct cifs_tcon *tcon, * will be closed when the caller closes the cached * handle. */ + cfid->has_lease = false; spin_unlock(&cfids->cfid_list_lock); kref_put(&cfid->refcount, smb2_close_cached_fid); goto out; } + spin_unlock(&cfids->cfid_list_lock); } - spin_unlock(&cfids->cfid_list_lock); +out: if (rc) { if (cfid->is_open) SMB2_close(0, cfid->tcon, cfid->fid.persistent_fid, cfid->fid.volatile_fid); free_cached_dir(cfid); - cfid = NULL; - } -out: - if (rc == 0) { + } else { *ret_cfid = cfid; atomic_inc(&tcon->num_remote_opens); } - + kfree(utf16_path); return rc; }