From mboxrd@z Thu Jan 1 00:00:00 1970 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 21ABEC001B0 for ; Wed, 16 Aug 2023 13:37:19 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S245579AbjHPNgs (ORCPT ); Wed, 16 Aug 2023 09:36:48 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:49174 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S245580AbjHPNgm (ORCPT ); Wed, 16 Aug 2023 09:36:42 -0400 Received: from sipsolutions.net (s3.sipsolutions.net [IPv6:2a01:4f8:242:246e::2]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id C55BB123 for ; Wed, 16 Aug 2023 06:36:41 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=sipsolutions.net; s=mail; h=MIME-Version:Content-Transfer-Encoding: Content-Type:References:In-Reply-To:Date:Cc:To:From:Subject:Message-ID:Sender :Reply-To:Content-ID:Content-Description:Resent-Date:Resent-From:Resent-To: Resent-Cc:Resent-Message-ID; bh=8jtHqQpmVWgSHJYBaqo9Xp/2soWaFrb2AmoKLrI+u54=; t=1692193001; x=1693402601; b=YO7jHwOmig7L3s7nR3q1eA9cL9GDUF5hkAXBt0LcD1FE7/8 5GDjmDkJm9zmOKTYEZwR9V9V+NBe0kU+sHapYnf+53gNlNSOHWx4fYFboMZmITL1Ug6eKWLmkk5R2 18j+cwuiXjbaMUCSjdmp8Ke4yuxuoPLfRsGCLN6sjNfiV9dds5HJE/HlXtv9THKSt80BuuPQpuv/y 4LVrtZC7GySJsKu/RHXzch26fV2S+1Za54h0pS4vuvA//9j1+XvArpZZ89BprH8bzoZkvri9XMR1T J57H8UmMxCltBDxcaakAasDf/SrjGkjg43TYuXehAwEj36YKdtIjbijhINmollyQ==; Received: by sipsolutions.net with esmtpsa (TLS1.3:ECDHE_X25519__RSA_PSS_RSAE_SHA256__AES_256_GCM:256) (Exim 4.96) (envelope-from ) id 1qWGhT-00AyDY-2d; Wed, 16 Aug 2023 15:36:40 +0200 Message-ID: <7bef5ebb985dacc7d497f2ab6d24a0f3402ce39a.camel@sipsolutions.net> Subject: Re: [RFC PATCH v3 6.1] wifi: cfg80211: fix cqm_config access race From: Johannes Berg To: linux-wireless@vger.kernel.org Cc: Max Schulze Date: Wed, 16 Aug 2023 15:36:38 +0200 In-Reply-To: <20230816153202.07a31b651a28.I1a1bb102329fc88e4712eaf394cba3025ada0dc7@changeid> References: <20230816153202.07a31b651a28.I1a1bb102329fc88e4712eaf394cba3025ada0dc7@changeid> Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable User-Agent: Evolution 3.48.4 (3.48.4-1.fc38) MIME-Version: 1.0 X-malware-bazaar: not-scanned Precedence: bulk List-ID: X-Mailing-List: linux-wireless@vger.kernel.org Oh, wait, there's another bug here ... One that makes more sense. On Wed, 2023-08-16 at 15:32 +0200, Johannes Berg wrote: >=20 > @@ -12629,6 +12630,7 @@ static int nl80211_set_cqm_rssi(struct genl_info = *info, > u32 hysteresis) > { > struct cfg80211_registered_device *rdev =3D info->user_ptr[0]; > + struct cfg80211_cqm_config *cqm_config =3D NULL, *old; > struct net_device *dev =3D info->user_ptr[1]; > struct wireless_dev *wdev =3D dev->ieee80211_ptr; > int i, err; > @@ -12646,10 +12648,6 @@ static int nl80211_set_cqm_rssi(struct genl_info= *info, > wdev->iftype !=3D NL80211_IFTYPE_P2P_CLIENT) > return -EOPNOTSUPP; > =20 > - wdev_lock(wdev); > - cfg80211_cqm_config_free(wdev); This used to NULL out the value on freeing > - wdev_unlock(wdev); > - > if (n_thresholds <=3D 1 && rdev->ops->set_cqm_rssi_config) { > if (n_thresholds =3D=3D 0 || thresholds[0] =3D=3D 0) /* Disabling */ > return rdev_set_cqm_rssi_config(rdev, dev, 0, 0); > @@ -12666,9 +12664,9 @@ static int nl80211_set_cqm_rssi(struct genl_info = *info, > n_thresholds =3D 0; > =20 > wdev_lock(wdev); > + old =3D rcu_dereference_protected(wdev->cqm_config, > + lockdep_is_held(&wdev->mtx)); > if (n_thresholds) { > - struct cfg80211_cqm_config *cqm_config; > - > cqm_config =3D kzalloc(struct_size(cqm_config, rssi_thresholds, > n_thresholds), > GFP_KERNEL); > @@ -12683,10 +12681,16 @@ static int nl80211_set_cqm_rssi(struct genl_inf= o *info, > flex_array_size(cqm_config, rssi_thresholds, > n_thresholds)); > =20 > - wdev->cqm_config =3D cqm_config; > + rcu_assign_pointer(wdev->cqm_config, cqm_config); > } > =20 > - err =3D cfg80211_cqm_rssi_update(rdev, dev); > + err =3D cfg80211_cqm_rssi_update(rdev, dev, cqm_config); > + if (err) { > + rcu_assign_pointer(wdev->cqm_config, old); > + kfree_rcu(cqm_config, rcu_head); > + } else { > + kfree_rcu(old, rcu_head); But I didn't put that here! So you obviously have UAF when removing a CQM config. johannes