From patchwork Tue Apr 25 19:26:06 2023 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Patchwork-Submitter: Fedor Pchelkin X-Patchwork-Id: 87533 Return-Path: Delivered-To: ouuuleilei@gmail.com Received: by 2002:a59:b0ea:0:b0:3b6:4342:cba0 with SMTP id b10csp3623915vqo; Tue, 25 Apr 2023 12:27:57 -0700 (PDT) X-Google-Smtp-Source: AKy350bQzRp10zJpkODpY72TjPxgspIN2eriQvkCUh9ZLR5ZqmR+8p0Jfpw/udEBId3DBwEAcBiD X-Received: by 2002:a05:6a21:6daa:b0:f3:81c1:feba with SMTP id wl42-20020a056a216daa00b000f381c1febamr12569995pzb.4.1682450877605; Tue, 25 Apr 2023 12:27:57 -0700 (PDT) ARC-Seal: i=1; a=rsa-sha256; t=1682450877; cv=none; d=google.com; s=arc-20160816; b=anWbOru2K19rQQcoW8J6Cy32Q0XkWt9vdsv/OoRBrB++T+FDLc4Vn4fcLT06rmNnSH iQdmuzL5BQh/IDa/BE3Isrb9UJaqfBiFdTDn0fhr2kh/qgXbCjXq/PrnMZK/xI+cTlVT rM73d7bNyAgada1NzwYwh8+7u3PgglLXoFd3utuXJlVSSDs5UpktkAb4EnLtokAM6dL6 PTzwbRalWH8H4DsjjNwlVkZe5G2qNI7hgGkphTV+xC9kf4kaOQiHHVR3VN+YqqPNmCGb tLUKKzXL6hIFdIXKITF/j04gNtxFeKXiAREo874vE49qW+OmHgcVANa5vmdsT+JtVfOR smLA== 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 :references:in-reply-to:message-id:date:subject:cc:to:from :dkim-signature:dkim-filter; bh=vtxoNftItfp9sdIuiR6TC+lugMM08soTNMG6xlHTUHY=; b=JM/PsevmVG/0+YcbeaGS1iPk1zJOhMmlw7GFIUJmQpMqUcZHBRW1dGht7LZ3vG7/nq aSAf4hC9qo1xpdJum500svhbaz/QruVnIinIDWnM/pIsz7XtOZrqREejnnsVedbCcodG r1ofD+xaepGTGOpRZD3kralndIUrQ+HIGCVfEP5yUqdhLZNi0XWnpsCxgYTyUq9mEsye 1lj+GFrVHOWIevBb4eegL4h4N1O39WtyVzSjTZpXmlR7SVpjI02+MPy2QDRwk5sah1Pk 5LnQwCRgCE5W1dA4e2vCNXVNWlpz9VqJPUO1ZsYQ6tHPVgCRZO82cQz3gaB7glQnB34H 8XEQ== ARC-Authentication-Results: i=1; mx.google.com; dkim=pass header.i=@ispras.ru header.s=default header.b="hHs4+//l"; 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=ispras.ru Received: from out1.vger.email (out1.vger.email. [2620:137:e000::1:20]) by mx.google.com with ESMTP id 10-20020a63124a000000b0051f0d0d6359si11034483pgs.118.2023.04.25.12.27.42; Tue, 25 Apr 2023 12:27:57 -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=@ispras.ru header.s=default header.b="hHs4+//l"; 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=ispras.ru Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S236040AbjDYT0h (ORCPT + 99 others); Tue, 25 Apr 2023 15:26:37 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:47340 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S236006AbjDYT0b (ORCPT ); Tue, 25 Apr 2023 15:26:31 -0400 Received: from mail.ispras.ru (mail.ispras.ru [83.149.199.84]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id 0B60516181; Tue, 25 Apr 2023 12:26:21 -0700 (PDT) Received: from fpc.intra.ispras.ru (unknown [10.10.165.4]) by mail.ispras.ru (Postfix) with ESMTPSA id AA7D440737A1; Tue, 25 Apr 2023 19:26:16 +0000 (UTC) DKIM-Filter: OpenDKIM Filter v2.11.0 mail.ispras.ru AA7D440737A1 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=ispras.ru; s=default; t=1682450776; bh=vtxoNftItfp9sdIuiR6TC+lugMM08soTNMG6xlHTUHY=; h=From:To:Cc:Subject:Date:In-Reply-To:References:From; b=hHs4+//l7vV8UdP9DwaVLuG8XDv+uZPdvuzCG+8MFti52uZty2gE84aRweqUZIFAQ b9RaIWs3ZkeIvOxVPB2gQ0Jjg2bCUCR5H5xy/LSRYOjFs1v4DeYERpO7JyRMgwnn9D 6nKCZLV/BySqjQd6RgrrvgrIklMSytHd5zxKyGLc= From: Fedor Pchelkin To: =?utf-8?q?Toke_H=C3=B8iland-J=C3=B8rgensen?= , Kalle Vallo Cc: Fedor Pchelkin , "David S. Miller" , Eric Dumazet , Jakub Kicinski , Paolo Abeni , Senthil Balasubramanian , "John W. Linville" , Vasanthakumar Thiagarajan , Sujith , linux-wireless@vger.kernel.org, netdev@vger.kernel.org, linux-kernel@vger.kernel.org, Alexey Khoroshilov , lvc-project@linuxtesting.org, syzbot+f2cb6e0ffdb961921e4d@syzkaller.appspotmail.com, Hillf Danton Subject: [PATCH v3 1/2] wifi: ath9k: fix races between ath9k_wmi_cmd and ath9k_wmi_ctrl_rx Date: Tue, 25 Apr 2023 22:26:06 +0300 Message-Id: <20230425192607.18015-1-pchelkin@ispras.ru> X-Mailer: git-send-email 2.34.1 In-Reply-To: <20230425033832.2041-1-hdanton@sina.com> References: MIME-Version: 1.0 X-Spam-Status: No, score=-2.1 required=5.0 tests=BAYES_00,DKIM_SIGNED, DKIM_VALID,DKIM_VALID_AU,DKIM_VALID_EF,SPF_HELO_NONE,SPF_PASS, T_SCC_BODY_TEXT_LINE,URIBL_BLOCKED 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?1764126371800470408?= X-GMAIL-MSGID: =?utf-8?q?1764177611441804311?= Currently, the synchronization between ath9k_wmi_cmd() and ath9k_wmi_ctrl_rx() is exposed to a race condition which, although being rather unlikely, can lead to invalid behaviour of ath9k_wmi_cmd(). Consider the following scenario: CPU0 CPU1 ath9k_wmi_cmd(...) mutex_lock(&wmi->op_mutex) ath9k_wmi_cmd_issue(...) wait_for_completion_timeout(...) --- timeout --- /* the callback is being processed * before last_seq_id became zero */ ath9k_wmi_ctrl_rx(...) spin_lock_irqsave(...) /* wmi->last_seq_id check here * doesn't detect timeout yet */ spin_unlock_irqrestore(...) /* last_seq_id is zeroed to * indicate there was a timeout */ wmi->last_seq_id = 0 mutex_unlock(&wmi->op_mutex) return -ETIMEDOUT ath9k_wmi_cmd(...) mutex_lock(&wmi->op_mutex) /* the buffer is replaced with * another one */ wmi->cmd_rsp_buf = rsp_buf wmi->cmd_rsp_len = rsp_len ath9k_wmi_cmd_issue(...) spin_lock_irqsave(...) spin_unlock_irqrestore(...) wait_for_completion_timeout(...) /* the continuation of the * callback left after the first * ath9k_wmi_cmd call */ ath9k_wmi_rsp_callback(...) /* copying data designated * to already timeouted * WMI command into an * inappropriate wmi_cmd_buf */ memcpy(...) complete(&wmi->cmd_wait) /* awakened by the bogus callback * => invalid return result */ mutex_unlock(&wmi->op_mutex) return 0 To fix this, update last_seq_id on timeout path inside ath9k_wmi_cmd() under the wmi_lock. Move ath9k_wmi_rsp_callback() under wmi_lock inside ath9k_wmi_ctrl_rx() so that the wmi->cmd_wait can be completed only for initially designated wmi_cmd call, otherwise the path would be rejected with last_seq_id check. Found by Linux Verification Center (linuxtesting.org) with Syzkaller. Fixes: fb9987d0f748 ("ath9k_htc: Support for AR9271 chipset.") Signed-off-by: Fedor Pchelkin Acked-by: Toke Høiland-Jørgensen --- v2: do not extract ath9k_wmi_rsp_callback() internals, rephrase description v3: per Hillf Danton's comment, protect last_seq_id with wmi_lock on timeout path, divide the v2 version into two separate patches; the first one is concerned with last_seq_id and completion under wmi_lock, the second one is for moving rsp buffer and length recording under wmi lock. Note that it's been only tested with Syzkaller and on basic driver scenarios with real hardware. drivers/net/wireless/ath/ath9k/wmi.c | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/drivers/net/wireless/ath/ath9k/wmi.c b/drivers/net/wireless/ath/ath9k/wmi.c index d652c647d56b..04f363cb90fe 100644 --- a/drivers/net/wireless/ath/ath9k/wmi.c +++ b/drivers/net/wireless/ath/ath9k/wmi.c @@ -242,10 +242,10 @@ static void ath9k_wmi_ctrl_rx(void *priv, struct sk_buff *skb, spin_unlock_irqrestore(&wmi->wmi_lock, flags); goto free_skb; } - spin_unlock_irqrestore(&wmi->wmi_lock, flags); /* WMI command response */ ath9k_wmi_rsp_callback(wmi, skb); + spin_unlock_irqrestore(&wmi->wmi_lock, flags); free_skb: kfree_skb(skb); @@ -308,8 +308,8 @@ int ath9k_wmi_cmd(struct wmi *wmi, enum wmi_cmd_id cmd_id, struct ath_common *common = ath9k_hw_common(ah); u16 headroom = sizeof(struct htc_frame_hdr) + sizeof(struct wmi_cmd_hdr); + unsigned long time_left, flags; struct sk_buff *skb; - unsigned long time_left; int ret = 0; if (ah->ah_flags & AH_UNPLUGGED) @@ -345,7 +345,9 @@ int ath9k_wmi_cmd(struct wmi *wmi, enum wmi_cmd_id cmd_id, if (!time_left) { ath_dbg(common, WMI, "Timeout waiting for WMI command: %s\n", wmi_cmd_to_name(cmd_id)); + spin_lock_irqsave(&wmi->wmi_lock, flags); wmi->last_seq_id = 0; + spin_unlock_irqrestore(&wmi->wmi_lock, flags); mutex_unlock(&wmi->op_mutex); return -ETIMEDOUT; }