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 27696C4332F for ; Thu, 20 Jan 2022 06:13:26 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1358672AbiATGNX (ORCPT ); Thu, 20 Jan 2022 01:13:23 -0500 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:59436 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S229451AbiATGNW (ORCPT ); Thu, 20 Jan 2022 01:13:22 -0500 Received: from mail.marcansoft.com (marcansoft.com [IPv6:2a01:298:fe:f::2]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id 82BBAC061574; Wed, 19 Jan 2022 22:13:22 -0800 (PST) Received: from [127.0.0.1] (localhost [127.0.0.1]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (4096 bits)) (No client certificate requested) (Authenticated sender: marcan@marcan.st) by mail.marcansoft.com (Postfix) with ESMTPSA id E5C2F4246A; Thu, 20 Jan 2022 06:13:13 +0000 (UTC) Subject: Re: [PATCH v3 3/9] brcmfmac: firmware: Do not crash on a NULL board_type To: Andy Shevchenko Cc: Kalle Valo , "David S. Miller" , Jakub Kicinski , Rob Herring , "Rafael J. Wysocki" , Len Brown , Arend van Spriel , Franky Lin , Hante Meuleman , Chi-hsien Lin , Wright Feng , Dmitry Osipenko , Sven Peter , Alyssa Rosenzweig , Mark Kettenis , =?UTF-8?B?UmFmYcWCIE1pxYJlY2tp?= , Pieter-Paul Giesberts , Linus Walleij , Hans de Goede , "John W. Linville" , "brian m. carlson" , "open list:TI WILINK WIRELES..." , netdev , devicetree , Linux Kernel Mailing List , ACPI Devel Maling List , "open list:BROADCOM BRCM80211 IEEE802.11n WIRELESS DRIVER" , SHA-cyfmac-dev-list@infineon.com References: <20220117142919.207370-1-marcan@marcan.st> <20220117142919.207370-4-marcan@marcan.st> From: Hector Martin Message-ID: Date: Thu, 20 Jan 2022 15:13:11 +0900 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:78.0) Gecko/20100101 Thunderbird/78.13.0 MIME-Version: 1.0 In-Reply-To: Content-Type: text/plain; charset=utf-8 Content-Language: es-ES Content-Transfer-Encoding: 7bit Precedence: bulk List-ID: X-Mailing-List: linux-wireless@vger.kernel.org On 20/01/2022 06.45, Andy Shevchenko wrote: > On Mon, Jan 17, 2022 at 4:30 PM Hector Martin wrote: >> >> This unbreaks support for USB devices, which do not have a board_type >> to create an alt_path out of and thus were running into a NULL >> dereference. > > In v5.16 we have two call sites: > > 1. > if (cur->type == BRCMF_FW_TYPE_NVRAM && fwctx->req->board_type) { > ... > alt_path = brcm_alt_fw_path(cur->path, fwctx->req->board_type); > > 2. > alt_path = brcm_alt_fw_path(first->path, fwctx->req->board_type); > if (alt_path) { > ... > > Looking at them I would rather expect to see (as a quick fix, the > better solution is to unify those call sites by splitting out a common > helper): > > if (fwctx->req->board_type) { > alt_path = brcm_alt_fw_path(first->path, fwctx->req->board_type); > else > alt_path = NULL; > ... > Since brcm_alt_fw_path can fail anyway, and its return value is already NULL-checked, it makes sense to propagate the NULL board_path there rather than doing it at all the callsites. That's a common pattern, e.g. the entire DT API is designed to accept NULL nodes. That does mean that the first callsite has a redundant NULL check, yes, but that doesn't hurt. This is all going to change with subsequent patches anyway; the point of this patch is just to fix the regression. -- Hector Martin (marcan@marcan.st) Public Key: https://mrcn.st/pub