Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::3:6 as permitted sender) client-ip=2620:137:e000::3:6;
Date:   Thu, 12 Oct 2023 16:40:46 -0700
From:   Dan Williams <dan.j.williams@intel.com>
To:     Michal Wilczynski <michal.wilczynski@intel.com>,
        <linux-acpi@vger.kernel.org>, <linux-kernel@vger.kernel.org>,
        <nvdimm@lists.linux.dev>
CC:     <rafael.j.wysocki@intel.com>, <andriy.shevchenko@intel.com>,
        <lenb@kernel.org>, <dan.j.williams@intel.com>,
        <vishal.l.verma@intel.com>, <ira.weiny@intel.com>,
        Michal Wilczynski <michal.wilczynski@intel.com>,
        "Andy Shevchenko" <andy.shevchenko@gmail.com>
Subject: RE: [PATCH v1 1/2] ACPI: NFIT: Fix memory leak, and local use of
 devm_*()
Message-ID: <652883fe10d38_780ef29459@dwillia2-xfh.jf.intel.com.notmuch>
References: <20230926184520.2239723-1-michal.wilczynski@intel.com>
 <20230926184520.2239723-2-michal.wilczynski@intel.com>
Content-Type: text/plain; charset="us-ascii"
Content-Disposition: inline
In-Reply-To: <20230926184520.2239723-2-michal.wilczynski@intel.com>
MIME-Version: 1.0
X-MS-Exchange-AntiSpam-MessageData-ChunkCount: 1
X-MS-Exchange-AntiSpam-MessageData-0: =?us-ascii?Q?+UPtIvrzsGbcrHaT6I2PYfK+Ljw+tLyr4oDw2ETRW1Hkcu663TFtCfgY2JYp?=
 =?us-ascii?Q?Gkc8PDRztpZxotWK4C8ipCaKhtPT4vR8U0wius+ZP8uKNRjfQ95scCozEj/X?=
 =?us-ascii?Q?uEtBFuRWKB4PqPttkLUcYaF0MeG4IPqlVEijxLv8g8aefKMCosHW7GQI/Cv7?=
 =?us-ascii?Q?zMHFG4z92VRAVlGXQQD2jr5MHCk0j0isEWzAJuhbJv/ef984jf+Z+kUWTLuV?=
 =?us-ascii?Q?40pf/zmPBE2fh32eFifkh37Zuk7SnnGqC/tauU4jLQWFVyfYiRN6XUPMjRcK?=
 =?us-ascii?Q?N7FwRI5qN/xM7EWMXbt5TfFh19KbTLMiw9d0CfxI46NWCxhABVdbXov18jrC?=
 =?us-ascii?Q?hSna80EFoqxz+piiXsPPu+4GnquJm7zHY7O2gFdwZbPcUaO0tCImVz7gXXTc?=
 =?us-ascii?Q?wh0Cs+6aMQ2bW0vABYLbafzk8pd84xmpbgEVLfpeRXOCT115B0VrjUPk8t07?=
 =?us-ascii?Q?xxw34W+8XzBg8loJtCInGgl2vOZ8cOc8h8ugXv+eHt6xJMB2db/06qkzvtk4?=
 =?us-ascii?Q?Oa/OH00Vz0FPwURIkehymabEpOk3uovzR/rlWDe9WmkvZUQ1yWcTNf07mhoA?=
 =?us-ascii?Q?kxVqGOMOzLrPHRD0utJgQ3MeHjaYpKvQAgsVgQI9NujgtNbbp0l08Z4mNAc3?=
 =?us-ascii?Q?KkhVJTAbzRxa4EHL1A55MT7Il5kOcKI0ipZJlXPJsKpGXeE8+RaiM2iicNU9?=
 =?us-ascii?Q?ScphirVc1jI1q9SzoDDWk/npFcNTLJwRkFcIqWlhbQGXz14rXvp9neHzwonz?=
 =?us-ascii?Q?Z+f9wyw19GDyCx7HRaKFaTsDp28IjZZy4ttVWfHPb3A3sSt68VpfLgF/JxOt?=
 =?us-ascii?Q?c2x/dCCmyBKhTB8Y2Dk8b7vp985hYexkRTMzAmZ/37UjPbqV7FlId8JbNwM/?=
 =?us-ascii?Q?6HkxyhSM5br+9/glDWxR+OpqqaW4UEWtPHtI8wERyy+YO9zwPF183nBnI+7e?=
 =?us-ascii?Q?XVOwoGxi8J7qv87HP5jdr5SCgZ7HG/GYvS0fxiPB6xPJC/3NkLu+l3ybY6jy?=
 =?us-ascii?Q?KUMKj56GH59pDErZsr7xnmMs90WgxWELFHfp66XzV08g/k/cG99hbOCUIzAi?=
 =?us-ascii?Q?22OIiMWyVVIQGLkc5pAefysLCPpW0SDY33ZIoQaspLpOKf8SVJC4l48S4Rei?=
 =?us-ascii?Q?IL6ySlTQ9aEvxcVfYj6fMcBKhWC/+70qYLPvd4AM9172f1i5OSt+YPPa2cpq?=
 =?us-ascii?Q?wZ/eG+dqSAkZtRJqGeTxS+UvqQTg0NPUInFaJ9BDE08GBQurQ+jrd+mcXtOi?=
 =?us-ascii?Q?fjW0ri5ImTAKsDdysmS+9wseRbmWGjZrrZhbvQA3MVkGWJEffTZxfzA0Ejva?=
 =?us-ascii?Q?fTs/J9CJcZ8YYZK9rgNsj4xaKDvubvLzfX6KowMutvtmNTF3qQOccWzMebtw?=
 =?us-ascii?Q?uKHrTz9qOMgclV+85rfUqJY+1hq63lamPDw86gGXjzCrTGlN4MYECmN9qIDZ?=
 =?us-ascii?Q?/Tv85Ehi01zIWzOeba2Qg2FTz2Nnkhjwz419s8NP29Zv1Qr5dXpuTk1ohbRV?=
 =?us-ascii?Q?REuvkZW7vC/LQLfTRHHY5UXH9lA1UKepATT0gnq8Vv4+LW1HSrvbFHhL1/K/?=
 =?us-ascii?Q?g9g4KKBegQ5QJhDqOQQTX49Cab3LRfoKR8LVYVObLuAhQWYQhQSFV/aAf+IN?=
 =?us-ascii?Q?Pw=3D=3D?=
X-MS-Exchange-CrossTenant-Network-Message-Id: 71c028dd-3602-40a1-9bfe-08dbcb7ca9e3
X-MS-Exchange-CrossTenant-AuthSource: SA3PR11MB8118.namprd11.prod.outlook.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 12 Oct 2023 23:40:49.1024
 (UTC)
X-MS-Exchange-CrossTenant-FromEntityHeader: Hosted
X-MS-Exchange-CrossTenant-Id: 46c98d88-e344-4ed4-8496-4ed7712e255d
X-MS-Exchange-CrossTenant-MailboxType: HOSTED
X-MS-Exchange-CrossTenant-UserPrincipalName: l8YwX6+qErjZPfJaDPQAHkHJKYvsVPyNq49p26cfemYon+Valh4aRhzzoSjL2oEKy1+XzacLzXmPPatYG/LZkibyAVde1sPDus6pLKdwSow=
X-MS-Exchange-Transport-CrossTenantHeadersStamped: SN7PR11MB7509
X-OriginatorOrg: intel.com
X-Spam-Status: No, score=-0.9 required=5.0 tests=DKIMWL_WL_HIGH,DKIM_SIGNED,
	DKIM_VALID,DKIM_VALID_AU,HEADER_FROM_DIFFERENT_DOMAINS,
	MAILING_LIST_MULTI,SPF_HELO_NONE,SPF_PASS autolearn=unavailable
	autolearn_force=no version=3.4.6
X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on pete.vger.email
Precedence: bulk
List-ID: <linux-kernel.vger.kernel.org>
X-Mailing-List: linux-kernel@vger.kernel.org
X-Greylist: Sender passed SPF test, not delayed by milter-greylist-4.6.4 (pete.vger.email [0.0.0.0]); Thu, 12 Oct 2023 16:41:28 -0700 (PDT)

Michal Wilczynski wrote:
> devm_*() family of functions purpose is managing memory attached to a
> device. So in general it should only be used for allocations that should
> last for the whole lifecycle of the device. This is not the case for
> acpi_nfit_init_interleave_set(). There are two allocations that are only
> used locally in this function. What's more - if the function exits on
> error path memory is never freed. It's still attached to dev and would
> be freed on device detach, so this leak could be called a 'local leak'.

This analysis is incorrect devm cleans up on driver ->probe() failure in
addition to ->remove(), and these error returns result in ->probe()
failures. No leak, i.e. this is not a fix.

The conversion to modern probe is ok if you want to resubmit that one
without this intervening change.