From nobody Thu Dec 18 14:29:40 2025 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 2579BC4332F for ; Wed, 13 Dec 2023 10:25:39 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S235332AbjLMKZb (ORCPT ); Wed, 13 Dec 2023 05:25:31 -0500 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:43568 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S235299AbjLMKZ2 (ORCPT ); Wed, 13 Dec 2023 05:25:28 -0500 Received: from us-smtp-delivery-124.mimecast.com (us-smtp-delivery-124.mimecast.com [170.10.129.124]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id D43F1AC for ; Wed, 13 Dec 2023 02:25:34 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1702463134; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version:content-type:content-type: content-transfer-encoding:content-transfer-encoding; bh=W/r5C+C8vvlrOFPoBxr3FLWNYuCGMCfVD68sW2OExkw=; b=cGs2ZXRC/tfiFkdKQSJOVOZBy8g13PtGzrH3vHG0cmccWD8zYtuCIl4BS5TylVo4xcCrtQ 6Bde+VFCe0CpBfdupvDzAfBkL118dZUOfEX2VmH829BYEy8ahRn2Ja/CF6ABTKwBTTghUW 19vmdAA6Xi4rjLHy2eYJyQXB7XGn5uA= Received: from mimecast-mx02.redhat.com (mx-ext.redhat.com [66.187.233.73]) by relay.mimecast.com with ESMTP with STARTTLS (version=TLSv1.3, cipher=TLS_AES_256_GCM_SHA384) id us-mta-517-xtXyTSkzPE-y2qrqBKbpBg-1; Wed, 13 Dec 2023 05:25:32 -0500 X-MC-Unique: xtXyTSkzPE-y2qrqBKbpBg-1 Received: from smtp.corp.redhat.com (int-mx08.intmail.prod.int.rdu2.redhat.com [10.11.54.8]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (2048 bits) server-digest SHA256) (No client certificate requested) by mimecast-mx02.redhat.com (Postfix) with ESMTPS id CCE8A3C2B601; Wed, 13 Dec 2023 10:25:31 +0000 (UTC) Received: from warthog.procyon.org.uk (unknown [10.42.28.2]) by smtp.corp.redhat.com (Postfix) with ESMTP id D1CB5C15A0C; Wed, 13 Dec 2023 10:25:30 +0000 (UTC) Organization: Red Hat UK Ltd. Registered Address: Red Hat UK Ltd, Amberley Place, 107-111 Peascod Street, Windsor, Berkshire, SI4 1TE, United Kingdom. Registered in England and Wales under Company Registration No. 3798903 From: David Howells To: Marc Dionne cc: dhowells@redhat.com, linux-afs@lists.infradead.org, linux-fsdevel@vger.kernel.org, linux-kernel@vger.kernel.org Subject: [PATCH] afs: Fix use-after-free due to get/remove race in volume tree MIME-Version: 1.0 Content-ID: <307295.1702463129.1@warthog.procyon.org.uk> Content-Transfer-Encoding: quoted-printable Date: Wed, 13 Dec 2023 10:25:29 +0000 Message-ID: <307296.1702463129@warthog.procyon.org.uk> X-Scanned-By: MIMEDefang 3.4.1 on 10.11.54.8 Precedence: bulk List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Type: text/plain; charset="utf-8" When an afs_volume struct is put, its refcount is reduced to 0 before the cell->volume_lock is taken and the volume removed from the cell->volumes tree. Unfortunately, this means that the lookup code can race and see a volume with a zero ref in the tree, resulting in a use-after-free: refcount_t: addition on 0; use-after-free. WARNING: CPU: 3 PID: 130782 at lib/refcount.c:25 refcount_warn_satu= rate+0x7a/0xda ... RIP: 0010:refcount_warn_saturate+0x7a/0xda ... Call Trace: ? __warn+0x8b/0xf5 ? report_bug+0xbf/0x11b ? refcount_warn_saturate+0x7a/0xda ? handle_bug+0x3c/0x5b ? exc_invalid_op+0x13/0x59 ? asm_exc_invalid_op+0x16/0x20 ? refcount_warn_saturate+0x7a/0xda ? refcount_warn_saturate+0x7a/0xda afs_get_volume+0x3d/0x55 afs_create_volume+0x126/0x1de afs_validate_fc+0xfe/0x130 afs_get_tree+0x20/0x2e5 vfs_get_tree+0x1d/0xc9 do_new_mount+0x13b/0x22e do_mount+0x5d/0x8a __do_sys_mount+0x100/0x12a do_syscall_64+0x3a/0x94 entry_SYSCALL_64_after_hwframe+0x62/0x6a Fix this by: (1) When putting, use a flag to indicate if the volume has been removed from the tree and skip the rb_erase if it has. (2) When looking up, use a conditional ref increment and if it fails because the refcount is 0, replace the node in the tree and set the removal flag. Fixes: 20325960f875 ("afs: Reorganise volume and server trees to be rooted = on the cell") Signed-off-by: David Howells cc: Marc Dionne cc: linux-afs@lists.infradead.org Reviewed-by: Jeffrey Altman --- fs/afs/internal.h | 2 ++ fs/afs/volume.c | 26 +++++++++++++++++++++++--- 2 files changed, 25 insertions(+), 3 deletions(-) diff --git a/fs/afs/internal.h b/fs/afs/internal.h index a812952be1c9..7385d62c8cf5 100644 --- a/fs/afs/internal.h +++ b/fs/afs/internal.h @@ -586,6 +586,7 @@ struct afs_volume { #define AFS_VOLUME_OFFLINE 4 /* - T if volume offline notice given */ #define AFS_VOLUME_BUSY 5 /* - T if volume busy notice given */ #define AFS_VOLUME_MAYBE_NO_IBULK 6 /* - T if some servers don't have Inli= neBulkStatus */ +#define AFS_VOLUME_RM_TREE 7 /* - Set if volume removed from cell->volumes= */ #ifdef CONFIG_AFS_FSCACHE struct fscache_volume *cache; /* Caching cookie */ #endif @@ -1513,6 +1514,7 @@ extern struct afs_vlserver_list *afs_extract_vlserver= _list(struct afs_cell *, extern struct afs_volume *afs_create_volume(struct afs_fs_context *); extern int afs_activate_volume(struct afs_volume *); extern void afs_deactivate_volume(struct afs_volume *); +bool afs_try_get_volume(struct afs_volume *volume, enum afs_volume_trace r= eason); extern struct afs_volume *afs_get_volume(struct afs_volume *, enum afs_vol= ume_trace); extern void afs_put_volume(struct afs_net *, struct afs_volume *, enum afs= _volume_trace); extern int afs_check_volume_status(struct afs_volume *, struct afs_operati= on *); diff --git a/fs/afs/volume.c b/fs/afs/volume.c index 29d483c80281..115c081a8e2c 100644 --- a/fs/afs/volume.c +++ b/fs/afs/volume.c @@ -32,8 +32,13 @@ static struct afs_volume *afs_insert_volume_into_cell(st= ruct afs_cell *cell, } else if (p->vid > volume->vid) { pp =3D &(*pp)->rb_right; } else { - volume =3D afs_get_volume(p, afs_volume_trace_get_cell_insert); - goto found; + if (afs_try_get_volume(p, afs_volume_trace_get_cell_insert)) { + volume =3D p; + goto found; + } + + set_bit(AFS_VOLUME_RM_TREE, &volume->flags); + rb_replace_node_rcu(&p->cell_node, &volume->cell_node, &cell->volumes); } } =20 @@ -56,7 +61,8 @@ static void afs_remove_volume_from_cell(struct afs_volume= *volume) afs_volume_trace_remove); write_seqlock(&cell->volume_lock); hlist_del_rcu(&volume->proc_link); - rb_erase(&volume->cell_node, &cell->volumes); + if (!test_and_set_bit(AFS_VOLUME_RM_TREE, &volume->flags)) + rb_erase(&volume->cell_node, &cell->volumes); write_sequnlock(&cell->volume_lock); } } @@ -231,6 +237,20 @@ static void afs_destroy_volume(struct afs_net *net, st= ruct afs_volume *volume) _leave(" [destroyed]"); } =20 +/* + * Try to get a reference on a volume record. + */ +bool afs_try_get_volume(struct afs_volume *volume, enum afs_volume_trace r= eason) +{ + int r; + + if (__refcount_inc_not_zero(&volume->ref, &r)) { + trace_afs_volume(volume->vid, r + 1, reason); + return true; + } + return false; +} + /* * Get a reference on a volume record. */