From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from smtpout-02.galae.net (smtpout-02.galae.net [185.246.84.56]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id 0165030EF63; Mon, 31 Aug 2026 07:45:33 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=185.246.84.56 ARC-Seal:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1788162336; cv=none; b=N76w5oMnam+cYl72L4owa6RZlIPo7TDhx1m+vF4HMUTAuMqaId9OwqIc47oD1Hl+IC/q+2OqvyKotVzUdxtN6L0xY0gKcGZ011hUGRVTcCvG1hJimUTG0jIP1AM9h03urJLkoVos+R6VyQQWVKyCkZDr7c7cn+RNnlwjXaV9XAY= ARC-Message-Signature:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1788162336; c=relaxed/simple; bh=0C1DpZSex7yIopHmCohYVtHcaaxa74ajHIR5lkcvaIQ=; h=From:To:Cc:Subject:In-Reply-To:References:Date:Message-ID: MIME-Version:Content-Type; b=qx3G+pfgPJH4r/RedoY9dXHFNtnOb6RTRV8qikHs7qXxi7C6R6dCwka2NE0wBXQePhvaJwWrp9WNEouHpfeWDZi6SnK0yPrcAqHxZpoioZ6kW6TnZBJZQ9+1eMkqsVPMQnNE6jHIy58G9a5cWB8CEq143ihqPW0vXFcuanKpVw0= ARC-Authentication-Results:i=1; smtp.subspace.kernel.org; dmarc=pass (p=reject dis=none) header.from=bootlin.com; spf=pass smtp.mailfrom=bootlin.com; dkim=pass (2048-bit key) header.d=bootlin.com header.i=@bootlin.com header.b=wLaMV6nv; arc=none smtp.client-ip=185.246.84.56 Authentication-Results: smtp.subspace.kernel.org; dmarc=pass (p=reject dis=none) header.from=bootlin.com Authentication-Results: smtp.subspace.kernel.org; spf=pass smtp.mailfrom=bootlin.com Authentication-Results: smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=bootlin.com header.i=@bootlin.com header.b="wLaMV6nv" Received: from smtpout-01.galae.net (smtpout-01.galae.net [212.83.139.233]) by smtpout-02.galae.net (Postfix) with ESMTPS id 53BB41A18E7; Mon, 31 Aug 2026 07:45:32 +0000 (UTC) Received: from mail.galae.net (mail.galae.net [212.83.136.155]) by smtpout-01.galae.net (Postfix) with ESMTPS id 1DF0F601E1; Mon, 31 Aug 2026 07:45:32 +0000 (UTC) Received: from [127.0.0.1] (localhost [127.0.0.1]) by localhost (Mailerdaemon) with ESMTPSA id B4F9611C783B9; Mon, 31 Aug 2026 09:45:20 +0200 (CEST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=bootlin.com; s=dkim; t=1788162327; h=from:subject:date:message-id:to:cc:mime-version:content-type: content-transfer-encoding:in-reply-to:references; bh=0C1DpZSex7yIopHmCohYVtHcaaxa74ajHIR5lkcvaIQ=; b=wLaMV6nvma89mGbAdovz6/agGE8q97+IlGEHAUuChQvNIN6cJE7Mqf7wSVSmK/v/jed3K7 FwEdYsOB9PsOusIAfGfHUMG0Jr7UK8bcqoRHHDnBxYEb6os7ZjxI+mktIfGaLoLQHyY4oK cUqBRNiVCWQBx7uRMxD8U/z/ckM+AAa0v/aQRlm0laqFYQzWeIJZIo5T/wQOu0CcFVqO8L Fqlz8LpIpQRs+weMyMI6Qvk9y4r3rKZLEqMGjWNUYTOSHNbDgnbJIj85RUivKpBY7mtf8j ZCu/ufza7jEF60YSKSqwvhHzw5cYt1TQBid7EvzQuRP5bFp8xoKWrnpB295PNg== From: Miquel Raynal To: Kaiwen Shi Cc: Alexander Aring , Stefan Schmidt , linux-wpan@vger.kernel.org, "David S . Miller" , Eric Dumazet , Jakub Kicinski , Paolo Abeni , Simon Horman , netdev@vger.kernel.org, linux-kernel@vger.kernel.org, Xuanqiang Luo , stable@vger.kernel.org Subject: Re: [PATCH net v4] mac802154: fix data race and NULL deref on local->assoc_dev In-Reply-To: <20260829230551.1787432-1-skwkevin@mail.ustc.edu.cn> (Kaiwen Shi's message of "Sun, 30 Aug 2026 07:05:51 +0800") References: <20260829230551.1787432-1-skwkevin@mail.ustc.edu.cn> User-Agent: mu4e 1.12.12; emacs 30.2 Date: Mon, 31 Aug 2026 09:45:19 +0200 Message-ID: <877bl6d980.fsf@bootlin.com> Precedence: bulk X-Mailing-List: linux-kernel@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: quoted-printable X-Last-TLS-Session-Version: TLSv1.3 On 30/08/2026 at 07:05:51 +08, Kaiwen Shi wrote: > local->assoc_dev is shared between the association path and the > association-response worker without common synchronization. > > mac802154_perform_association() stores the coordinator pointer and waits > for a response. Its timeout and error paths clear the pointer and return > to mac802154_associate(), which may then free the coordinator object. > Meanwhile, mac802154_rx_mac_cmd_worker() may observe the associating bit > and enter mac802154_process_association_resp(), which dereferences > assoc_dev. > > The worker's bit test and the handler's pointer dereference are not > atomic with respect to cleanup. Cleanup can clear assoc_dev between them, > causing a NULL dereference, or free the coordinator while the response > handler still uses the pointer. > > The recorded result is exposed to the same window. assoc_status and > assoc_addr are written by the handler but read by the association path > while the associating bit is still set, so a second response for the same > request - a malicious one, for instance - can replace them between those > reads and leave the caller with an incoherent status and address pair. > > The response handler only needs the coordinator extended address. > Replace assoc_dev with a cached address, removing the pointer lifetime > dependency. Protect the cached address and the associating bit with a > dedicated spinlock. A READ_ONCE()/WRITE_ONCE() pair would not guarantee > an atomic __le64 access on all 32-bit architectures. > > wpan_dev->association_lock cannot be reused here: nl802154_associate() > holds it across rdev_associate(), hence for the whole of > mac802154_perform_association() including the wait for the response. > A response handler taking that lock would only get it once the > association has already given up. > > Reset the completion, publish the cached address, and set the associating > bit while holding the lock. The response handler takes the lock, rechecks > the bit and the cached address, records the response, clears the bit, and > only then completes the waiter. Thus cleanup cannot pass the handler > between its state check and completion, and the cached 64-bit value > cannot tear. > > The handler clears the bit before completing, not the woken waiter: > otherwise complete() is issued under the lock and a second (e.g. > malicious) response can reacquire it before the waiter and replace the > result. So a wait that returns success implies the bit is already clear, > and the success and negative-response paths return directly. The > transmit-error and timeout paths still clear it under assoc_lock, which > serializes any racing response against the cleanup while the call returns > the error it already selected. Both paths snapshot assoc_status and > assoc_addr under the same lock. > > Both users run in process context, so a plain spinlock is sufficient. > The lock is not held while waiting for the completion. > > Suggested-by: Miquel Raynal > Suggested-by: Xuanqiang Luo > Fixes: fefd19807fe9 ("mac802154: Handle associating") > Cc: stable@vger.kernel.org > Signed-off-by: Kaiwen Shi > --- Reviewed-by: Miquel Raynal Thanks, Miqu=C3=A8l