From patchwork Thu Oct 13 17:53:08 2022 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Greg KH X-Patchwork-Id: 2252 Return-Path: Delivered-To: ouuuleilei@gmail.com Received: by 2002:a5d:4ac7:0:0:0:0:0 with SMTP id y7csp413734wrs; Thu, 13 Oct 2022 11:13:47 -0700 (PDT) X-Google-Smtp-Source: AMsMyM6KXbQRInXlMXCQZSKt/1pPaibq1vTPjxqwRDniBMA85vmYOhPv8aliNs/EASGsmnriEaIm X-Received: by 2002:a17:902:bcc4:b0:181:899a:ac9c with SMTP id o4-20020a170902bcc400b00181899aac9cmr940012pls.124.1665684827422; Thu, 13 Oct 2022 11:13:47 -0700 (PDT) ARC-Seal: i=1; a=rsa-sha256; t=1665684827; cv=none; d=google.com; s=arc-20160816; b=RuSg+DdVSDXKW68czG1jyfvzcf1IMEIvn7fgo35XKksC+dCDNRecGyiB9uQeVXJOE9 bBMFTs4CA1A4qH8PqnQ4a9bMYGk3uTJui7OOHgoP6z638INkkbfK06MqzI3ePoI6IM1w yW8i3XeRzCQS4SX4c0YNr6msB3+2J8s03haqaTaR466sXNL+XSIa+l4PGTxGE4vXvibW kWqxDlaXacMVhKbF/D7nbCrn7vuELrjqbflY8v0f31l7wFgJne3j/y5QoN6uMpU169au oH9WQRusVTW1hYe4Fkm5vjjApkEWhuVNWhnBSvKgvW/Iwx5uBVmgiGWija2938ukEva/ sAZg== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=arc-20160816; h=list-id:precedence:content-transfer-encoding:mime-version :user-agent:references:in-reply-to:message-id:date:subject:cc:to :from:dkim-signature; bh=qaWh7cQqMCexGiyovsFDcs0CMJdtexthRqEjDGfr+jY=; b=TycBb3skIbTVjpUFfZw/wklBTMYAU91sj+vCoiHm3Z8IF2g+TBYteM7nVnhFkGwwgu qTwu4nMYL72+0DaCKhquH8G2rlQDOv5OE14d+gZ+4D2eetjLWQGcB5eA0r+VMRvct9NF egQQvjg/Wv9v1t3Lb5n6db9U4DIDnhbE5vgLi5BSCqRh50LupCUvGJ5uB+bhEeM19CNP e/J9DqWk6tUi+Zh5eRB/amm53u/wttsss9Lp5ECt+EMz5TJ+RPNhQBhW0qQObxeLjI6a EqX6xzLVwQdzeOCDp12o54GMT5V0oJRK1YaJ8Z+iGhzRlLt5D7STzqh8wD2F3t76A8sI Yi+Q== ARC-Authentication-Results: i=1; mx.google.com; dkim=pass header.i=@linuxfoundation.org header.s=korg header.b=Mg6F2PMg; spf=pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org; dmarc=pass (p=NONE sp=NONE dis=NONE) header.from=linuxfoundation.org Received: from out1.vger.email (out1.vger.email. [2620:137:e000::1:20]) by mx.google.com with ESMTP id r10-20020a1709028bca00b001789a178e33si326431plo.428.2022.10.13.11.13.34; Thu, 13 Oct 2022 11:13:47 -0700 (PDT) Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20; Authentication-Results: mx.google.com; dkim=pass header.i=@linuxfoundation.org header.s=korg header.b=Mg6F2PMg; spf=pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org; dmarc=pass (p=NONE sp=NONE dis=NONE) header.from=linuxfoundation.org Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S231476AbiJMSNT (ORCPT + 99 others); Thu, 13 Oct 2022 14:13:19 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:34316 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S231769AbiJMSMa (ORCPT ); Thu, 13 Oct 2022 14:12:30 -0400 Received: from ams.source.kernel.org (ams.source.kernel.org [145.40.68.75]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id 5CBA3170DCC; Thu, 13 Oct 2022 11:09:19 -0700 (PDT) Received: from smtp.kernel.org (relay.kernel.org [52.25.139.140]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by ams.source.kernel.org (Postfix) with ESMTPS id 003F2B820CB; Thu, 13 Oct 2022 18:02:33 +0000 (UTC) Received: by smtp.kernel.org (Postfix) with ESMTPSA id 2F70DC433C1; Thu, 13 Oct 2022 18:02:31 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=linuxfoundation.org; s=korg; t=1665684151; bh=cLMk1ar/nQON1mZgjjsdpVy1arXfNUVtd1FNGe7wHxc=; h=From:To:Cc:Subject:Date:In-Reply-To:References:From; b=Mg6F2PMgkRvjBgb7z6ZNUS4aU5aXRC+Rglz7+CORq8TFS29/6dw+IKg7sUwfP2c/Z /Ci0tHCvtu3u/s93mL3YBUkRd/Ww7LeuVCE0NiPH4sD00knBGUQei87+FwVRjCXN52 zxdMspKzaKmt/VlwLVDFYKm7g3B5R3xT0WvFONE0= From: Greg Kroah-Hartman To: linux-kernel@vger.kernel.org Cc: Greg Kroah-Hartman , stable@vger.kernel.org, butt3rflyh4ck , Jeremy Kerr , "David S. Miller" Subject: [PATCH 6.0 30/34] mctp: prevent double key removal and unref Date: Thu, 13 Oct 2022 19:53:08 +0200 Message-Id: <20221013175147.291795983@linuxfoundation.org> X-Mailer: git-send-email 2.38.0 In-Reply-To: <20221013175146.507746257@linuxfoundation.org> References: <20221013175146.507746257@linuxfoundation.org> User-Agent: quilt/0.67 MIME-Version: 1.0 X-Spam-Status: No, score=-7.1 required=5.0 tests=BAYES_00,DKIMWL_WL_HIGH, DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,DKIM_VALID_EF,RCVD_IN_DNSWL_HI, SPF_HELO_NONE,SPF_PASS autolearn=ham autolearn_force=no version=3.4.6 X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on lindbergh.monkeyblade.net Precedence: bulk List-ID: X-Mailing-List: linux-kernel@vger.kernel.org X-getmail-retrieved-from-mailbox: =?utf-8?q?INBOX?= X-GMAIL-THRID: =?utf-8?q?1746597133479519179?= X-GMAIL-MSGID: =?utf-8?q?1746597133479519179?= From: Jeremy Kerr commit 3a732b46736cd8a29092e4b0b1a9ba83e672bf89 upstream. Currently, we have a bug where a simultaneous DROPTAG ioctl and socket close may race, as we attempt to remove a key from lists twice, and perform an unref for each removal operation. This may result in a uaf when we attempt the second unref. This change fixes the race by making __mctp_key_remove tolerant to being called on a key that has already been removed from the socket/net lists, and only performs the unref when we do the actual remove. We also need to hold the list lock on the ioctl cleanup path. This fix is based on a bug report and comprehensive analysis from butt3rflyh4ck , found via syzkaller. Cc: stable@vger.kernel.org Fixes: 63ed1aab3d40 ("mctp: Add SIOCMCTP{ALLOC,DROP}TAG ioctls for tag control") Reported-by: butt3rflyh4ck Signed-off-by: Jeremy Kerr Signed-off-by: David S. Miller Signed-off-by: Greg Kroah-Hartman --- net/mctp/af_mctp.c | 23 ++++++++++++++++------- net/mctp/route.c | 10 +++++----- 2 files changed, 21 insertions(+), 12 deletions(-) --- a/net/mctp/af_mctp.c +++ b/net/mctp/af_mctp.c @@ -295,11 +295,12 @@ __must_hold(&net->mctp.keys_lock) mctp_dev_release_key(key->dev, key); spin_unlock_irqrestore(&key->lock, flags); - hlist_del(&key->hlist); - hlist_del(&key->sklist); - - /* unref for the lists */ - mctp_key_unref(key); + if (!hlist_unhashed(&key->hlist)) { + hlist_del_init(&key->hlist); + hlist_del_init(&key->sklist); + /* unref for the lists */ + mctp_key_unref(key); + } kfree_skb(skb); } @@ -373,9 +374,17 @@ static int mctp_ioctl_alloctag(struct mc ctl.tag = tag | MCTP_TAG_OWNER | MCTP_TAG_PREALLOC; if (copy_to_user((void __user *)arg, &ctl, sizeof(ctl))) { - spin_lock_irqsave(&key->lock, flags); - __mctp_key_remove(key, net, flags, MCTP_TRACE_KEY_DROPPED); + unsigned long fl2; + /* Unwind our key allocation: the keys list lock needs to be + * taken before the individual key locks, and we need a valid + * flags value (fl2) to pass to __mctp_key_remove, hence the + * second spin_lock_irqsave() rather than a plain spin_lock(). + */ + spin_lock_irqsave(&net->mctp.keys_lock, flags); + spin_lock_irqsave(&key->lock, fl2); + __mctp_key_remove(key, net, fl2, MCTP_TRACE_KEY_DROPPED); mctp_key_unref(key); + spin_unlock_irqrestore(&net->mctp.keys_lock, flags); return -EFAULT; } --- a/net/mctp/route.c +++ b/net/mctp/route.c @@ -228,12 +228,12 @@ __releases(&key->lock) if (!key->manual_alloc) { spin_lock_irqsave(&net->mctp.keys_lock, flags); - hlist_del(&key->hlist); - hlist_del(&key->sklist); + if (!hlist_unhashed(&key->hlist)) { + hlist_del_init(&key->hlist); + hlist_del_init(&key->sklist); + mctp_key_unref(key); + } spin_unlock_irqrestore(&net->mctp.keys_lock, flags); - - /* unref for the lists */ - mctp_key_unref(key); } /* and one for the local reference */