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 6FCD4C433F5 for ; Fri, 28 Jan 2022 16:12:07 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1349670AbiA1QMG (ORCPT ); Fri, 28 Jan 2022 11:12:06 -0500 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:38848 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S239183AbiA1QMG (ORCPT ); Fri, 28 Jan 2022 11:12:06 -0500 Received: from mail-pj1-x1036.google.com (mail-pj1-x1036.google.com [IPv6:2607:f8b0:4864:20::1036]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id 8A097C061714; Fri, 28 Jan 2022 08:12:05 -0800 (PST) Received: by mail-pj1-x1036.google.com with SMTP id s2-20020a17090ad48200b001b501977b23so11345149pju.2; Fri, 28 Jan 2022 08:12:05 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20210112; h=message-id:date:mime-version:user-agent:subject:content-language:to :cc:references:from:in-reply-to:content-transfer-encoding; bh=T0gAqCfOxCNh3Tki5mXje3PXqfET1SNQsXyN2HdLLdE=; b=o4vTZcMdl7aQ5CinIIevTCCp1Vvw+vsssds9tjxQW3nKc8CD5CjVfz2KVEwe8BHbTV tA/hTywAnOPEjUgWS4BAA7fQbw/DSChEe0Nw9sCiPoMLfvS14uTisjISopb/qBTf6ID5 VxFDf7vUJEO4PrWkQ+3zTGhaGkCpYJF22MNW3Bnf60GpuBC7rjgS9ufOLpXOr2/AzpM9 cog9GtKm8stFUxhWyGbd1rfqjJ3X6RJSOz96H0t1pGATrwUl5jP/BnqMbrHB/Itw7om2 CF1+GfxV0W3WaRRabruZ6+rr29hQcPwY1VNWDZWst1AYCQixQZQLNGoZ14WwXvPvarDN 7NRQ== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=x-gm-message-state:message-id:date:mime-version:user-agent:subject :content-language:to:cc:references:from:in-reply-to :content-transfer-encoding; bh=T0gAqCfOxCNh3Tki5mXje3PXqfET1SNQsXyN2HdLLdE=; b=I1f67TqYbaI656F0WQoI0QzVGtI0fAO5KlDI98URYszzIxav4PxD1PbJmC4UXgH26R LPqFcHtjh2ZGhmgzevy1oSWEHz3Jpj1TCRaj/hfdkFO/v6EwNf6QYEKHmnj6tm1u2dK6 YKDlZc8Dgt+BQhcYyty7iSQNBDEXpWTYtHcKLPDUT/Q74TLtoLrxZmw9Aw4iaL18ii+8 Olmzvle27C1WZXdwH8+mjHBaMYJ6JI6qbQO3rcJxKQ5Zp5jyRSPHGAWmoOHsA5cHo2A+ YdpruYOUDzsOsMH7qYNnDqX/L4WPvx9EzY9ZspHkzlXw6fzx21J6u7TskdP2Fl7kgHRE 9a1g== X-Gm-Message-State: AOAM531j2Y1Xujl0D8e3WibmarMFNqRf317VBNu87vbfSW90uebHcHqe N304wM2g436Oej+PjsvNJPM= X-Google-Smtp-Source: ABdhPJyr+/E6lg9lmSxPC+5Zd4f66JpwmC73WNLq4sP3COaactSNFP3PPgMKohlTGYJLkfs3sF/eww== X-Received: by 2002:a17:902:ec86:: with SMTP id x6mr9326466plg.96.1643386325042; Fri, 28 Jan 2022 08:12:05 -0800 (PST) Received: from [192.168.1.3] (ip72-194-116-95.oc.oc.cox.net. [72.194.116.95]) by smtp.gmail.com with ESMTPSA id y13sm10026244pfi.2.2022.01.28.08.12.03 (version=TLS1_3 cipher=TLS_AES_128_GCM_SHA256 bits=128/128); Fri, 28 Jan 2022 08:12:04 -0800 (PST) Message-ID: Date: Fri, 28 Jan 2022 08:12:02 -0800 MIME-Version: 1.0 User-Agent: Mozilla/5.0 (Windows NT 10.0; Win64; x64; rv:91.0) Gecko/20100101 Thunderbird/91.5.0 Subject: Re: [PATCH] pinctrl: bcm2835: Fix a few error paths Content-Language: en-US To: Andy Shevchenko , Florian Fainelli Cc: linux-kernel@vger.kernel.org, Linus Walleij , Ray Jui , Scott Branden , "maintainer:BROADCOM BCM281XX/BCM11XXX/BCM216XX ARM ARCHITE..." , Nicolas Saenz Julienne , Phil Elwell , Krzysztof Kozlowski , Hans Verkuil , Jason Wang , Marc Zyngier , "open list:PIN CONTROL SUBSYSTEM" , "moderated list:BROADCOM BCM2711/BCM2835 ARM ARCHITECTURE" , "moderated list:BROADCOM BCM2711/BCM2835 ARM ARCHITECTURE" References: <20220127215033.267227-1-f.fainelli@gmail.com> From: Florian Fainelli In-Reply-To: Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 7bit Precedence: bulk List-ID: X-Mailing-List: linux-gpio@vger.kernel.org On 1/28/2022 6:35 AM, Andy Shevchenko wrote: > On Thu, Jan 27, 2022 at 01:50:31PM -0800, Florian Fainelli wrote: >> After commit 266423e60ea1 ("pinctrl: bcm2835: Change init order for gpio >> hogs") a few error paths would not unwind properly the registration of >> gpio ranges. Correct that by assigning a single error label and goto it >> whenever we encounter a fatal error. > >> 1 file changed, 15 insertions(+), 8 deletions(-) > > While this seems legit per se, my eyes caught this: > > >> if (!girq->parents) { >> - pinctrl_remove_gpio_range(pc->pctl_dev, &pc->gpio_range); >> - return -ENOMEM; >> + err = -ENOMEM; >> + goto out_remove; > > Non-devm.... > >> } >> >> if (is_7211) { >> pc->wake_irq = devm_kcalloc(dev, BCM2835_NUM_IRQS, >> sizeof(*pc->wake_irq), >> GFP_KERNEL); > > ...followed by devm. > > It means more ordering bugs in the ->remove() and error path are lurking > around. Can you double check and be sure that we do not have a case where > non-devm registration code followed by devm? It seems to me like we are fine with the patch as is, because: - girq->parents is allocated with devm - pc->wake_irq is allocated with devm - name is allocated with devm and those are the only variables being allocated for which we also process an error handling path. -- Florian 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 bombadil.infradead.org (bombadil.infradead.org [198.137.202.133]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.lore.kernel.org (Postfix) with ESMTPS id 78C09C433F5 for ; Fri, 28 Jan 2022 16:14:21 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=lists.infradead.org; s=bombadil.20210309; h=Sender:Content-Type: Content-Transfer-Encoding:List-Subscribe:List-Help:List-Post:List-Archive: List-Unsubscribe:List-Id:In-Reply-To:From:References:Cc:To:Subject: MIME-Version:Date:Message-ID:Reply-To:Content-ID:Content-Description: Resent-Date:Resent-From:Resent-Sender:Resent-To:Resent-Cc:Resent-Message-ID: List-Owner; bh=XbORKWmVGHLSRLp5LlvFs4mnCrD9/i7uYC+A0xa8k1Y=; b=khfrSxodRY5R3o 5nCz0AfA46KfLloYkfWdj04Iy2XYwmDM2+yCt5imAC9FWOPsS44iXTsaVJSnJN/OkrOfoV/tA48/Q x0ZvRB8Na7EoylOir+ZhfYAGH4HgiIcMRm+KNt3l8QoWJ8qg7ilgYSqX8lG3z67WadzHtx17wJcbU M8rY12nBKfn+xJfCuWMnNB7W8Ok/QvFMFjmHFK8CeNIYsCvJuSGJtYpMxhHNyorK6mqKd5LoPJi41 XMj9K3XDP/EsO+fSpkmmj13NK1Y7Z3fyLtEYyY7gLQV7flEzXQOVdN24iUbet8EnPF3oo08bBh05a aacvMFV1l8OmcKq8esYg==; Received: from localhost ([::1] helo=bombadil.infradead.org) by bombadil.infradead.org with esmtp (Exim 4.94.2 #2 (Red Hat Linux)) id 1nDTrK-002tYD-1w; Fri, 28 Jan 2022 16:12:24 +0000 Received: from mail-pj1-x102b.google.com ([2607:f8b0:4864:20::102b]) by bombadil.infradead.org with esmtps (Exim 4.94.2 #2 (Red Hat Linux)) id 1nDTr6-002tTn-FC; Fri, 28 Jan 2022 16:12:09 +0000 Received: by mail-pj1-x102b.google.com with SMTP id l24-20020a17090aec1800b001b55738f633so7518319pjy.1; Fri, 28 Jan 2022 08:12:05 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20210112; h=message-id:date:mime-version:user-agent:subject:content-language:to :cc:references:from:in-reply-to:content-transfer-encoding; bh=T0gAqCfOxCNh3Tki5mXje3PXqfET1SNQsXyN2HdLLdE=; b=o4vTZcMdl7aQ5CinIIevTCCp1Vvw+vsssds9tjxQW3nKc8CD5CjVfz2KVEwe8BHbTV tA/hTywAnOPEjUgWS4BAA7fQbw/DSChEe0Nw9sCiPoMLfvS14uTisjISopb/qBTf6ID5 VxFDf7vUJEO4PrWkQ+3zTGhaGkCpYJF22MNW3Bnf60GpuBC7rjgS9ufOLpXOr2/AzpM9 cog9GtKm8stFUxhWyGbd1rfqjJ3X6RJSOz96H0t1pGATrwUl5jP/BnqMbrHB/Itw7om2 CF1+GfxV0W3WaRRabruZ6+rr29hQcPwY1VNWDZWst1AYCQixQZQLNGoZ14WwXvPvarDN 7NRQ== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=x-gm-message-state:message-id:date:mime-version:user-agent:subject :content-language:to:cc:references:from:in-reply-to :content-transfer-encoding; bh=T0gAqCfOxCNh3Tki5mXje3PXqfET1SNQsXyN2HdLLdE=; b=f8hINP4O2jZwIuLSScQ8Cl9i8z/WmpeJ9b5W+8ghId31Fb2fAoF5E0uB3K2Ponvzsb JH/dgzd+4DeFxM14l0ZK1oLIiIuo3LHNp9nh7RZObwXsp+fVw8mpTM6DKCz1fLQ/mJTo o6rZavAhrg7Ddt1aanuxurEvXXSrhwhUCzI7NCb6nUxkN9mUXr+DXCYmWGJ2Yp2G4hlw bZGpvvFQtqBI3odApT/Lyx10CLbcBYOulZOdLodz8Cip8BhlJDcWOxEwl8nmyDqm7xRX 2aKfSq4WNvVCieWtFubOKymJhRJSNTz7QBxXbvkmLdSB69Il8iMz0gNDUA0QVtBFydak R6Tw== X-Gm-Message-State: AOAM533Mn1LU745UsPMY/q7F8e2uhzvpw70CkrYHVzkgnQ5UeUDmFmDl L3N5hBUoFMMaVJ9CCwXL2UI= X-Google-Smtp-Source: ABdhPJyr+/E6lg9lmSxPC+5Zd4f66JpwmC73WNLq4sP3COaactSNFP3PPgMKohlTGYJLkfs3sF/eww== X-Received: by 2002:a17:902:ec86:: with SMTP id x6mr9326466plg.96.1643386325042; Fri, 28 Jan 2022 08:12:05 -0800 (PST) Received: from [192.168.1.3] (ip72-194-116-95.oc.oc.cox.net. [72.194.116.95]) by smtp.gmail.com with ESMTPSA id y13sm10026244pfi.2.2022.01.28.08.12.03 (version=TLS1_3 cipher=TLS_AES_128_GCM_SHA256 bits=128/128); Fri, 28 Jan 2022 08:12:04 -0800 (PST) Message-ID: Date: Fri, 28 Jan 2022 08:12:02 -0800 MIME-Version: 1.0 User-Agent: Mozilla/5.0 (Windows NT 10.0; Win64; x64; rv:91.0) Gecko/20100101 Thunderbird/91.5.0 Subject: Re: [PATCH] pinctrl: bcm2835: Fix a few error paths Content-Language: en-US To: Andy Shevchenko , Florian Fainelli Cc: linux-kernel@vger.kernel.org, Linus Walleij , Ray Jui , Scott Branden , "maintainer:BROADCOM BCM281XX/BCM11XXX/BCM216XX ARM ARCHITE..." , Nicolas Saenz Julienne , Phil Elwell , Krzysztof Kozlowski , Hans Verkuil , Jason Wang , Marc Zyngier , "open list:PIN CONTROL SUBSYSTEM" , "moderated list:BROADCOM BCM2711/BCM2835 ARM ARCHITECTURE" , "moderated list:BROADCOM BCM2711/BCM2835 ARM ARCHITECTURE" References: <20220127215033.267227-1-f.fainelli@gmail.com> From: Florian Fainelli In-Reply-To: X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20220128_081208_559136_9B8A0F7E X-CRM114-Status: GOOD ( 12.83 ) X-BeenThere: linux-arm-kernel@lists.infradead.org X-Mailman-Version: 2.1.34 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Content-Transfer-Encoding: 7bit Content-Type: text/plain; charset="us-ascii"; Format="flowed" Sender: "linux-arm-kernel" Errors-To: linux-arm-kernel-bounces+linux-arm-kernel=archiver.kernel.org@lists.infradead.org On 1/28/2022 6:35 AM, Andy Shevchenko wrote: > On Thu, Jan 27, 2022 at 01:50:31PM -0800, Florian Fainelli wrote: >> After commit 266423e60ea1 ("pinctrl: bcm2835: Change init order for gpio >> hogs") a few error paths would not unwind properly the registration of >> gpio ranges. Correct that by assigning a single error label and goto it >> whenever we encounter a fatal error. > >> 1 file changed, 15 insertions(+), 8 deletions(-) > > While this seems legit per se, my eyes caught this: > > >> if (!girq->parents) { >> - pinctrl_remove_gpio_range(pc->pctl_dev, &pc->gpio_range); >> - return -ENOMEM; >> + err = -ENOMEM; >> + goto out_remove; > > Non-devm.... > >> } >> >> if (is_7211) { >> pc->wake_irq = devm_kcalloc(dev, BCM2835_NUM_IRQS, >> sizeof(*pc->wake_irq), >> GFP_KERNEL); > > ...followed by devm. > > It means more ordering bugs in the ->remove() and error path are lurking > around. Can you double check and be sure that we do not have a case where > non-devm registration code followed by devm? It seems to me like we are fine with the patch as is, because: - girq->parents is allocated with devm - pc->wake_irq is allocated with devm - name is allocated with devm and those are the only variables being allocated for which we also process an error handling path. -- Florian _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel