Re: [PATCH AUTOSEL 6.1 13/22] proc/vmcore: fix potential memory leak in vmcore_init()

From: Sasha Levin <sashal@kernel.org>
To: Andrew Morton <akpm@linux-foundation.org>
Cc: linux-kernel@vger.kernel.org, stable@vger.kernel.org,
	Jianglei Nie <niejianglei2021@163.com>,
	Baoquan He <bhe@redhat.com>,
	Benjamin Herrenschmidt <benh@kernel.crashing.org>,
	Chen Lifu <chenlifu@huawei.com>,
	"Eric W . Biederman" <ebiederm@xmission.com>,
	Li Chen <lchen@ambarella.com>,
	Michael Ellerman <mpe@ellerman.id.au>,
	Paul Mackerras <paulus@samba.org>, Petr Mladek <pmladek@suse.com>,
	Russell King <linux@armlinux.org.uk>,
	ye xingchen <ye.xingchen@zte.com.cn>,
	Zeal Robot <zealci@zte.com.cn>,
	kexec@lists.infradead.org, linux-fsdevel@vger.kernel.org,
	Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Subject: Re: [PATCH AUTOSEL 6.1 13/22] proc/vmcore: fix potential memory leak in vmcore_init()
Date: Sun, 18 Dec 2022 06:28:16 -0500	[thread overview]
Message-ID: <Y575UCk+lwfJ2CoE@sashalap> (raw)
In-Reply-To: <20221217163228.9308c293458ceb680a6ffed8@linux-foundation.org>

On Sat, Dec 17, 2022 at 04:32:28PM -0800, Andrew Morton wrote:
>On Sat, 17 Dec 2022 10:27:14 -0500 Sasha Levin <sashal@kernel.org> wrote:
>
>> From: Jianglei Nie <niejianglei2021@163.com>
>>
>> [ Upstream commit 12b9d301ff73122aebd78548fa4c04ca69ed78fe ]
>>
>> Patch series "Some minor cleanup patches resent".
>>
>> The first three patches trivial clean up patches.
>>
>> And for the patch "kexec: replace crash_mem_range with range", I got a
>> ibm-p9wr ppc64le system to test, it works well.
>>
>> This patch (of 4):
>>
>> elfcorehdr_alloc() allocates a memory chunk for elfcorehdr_addr with
>> kzalloc().  If is_vmcore_usable() returns false, elfcorehdr_addr is a
>> predefined value.  If parse_crash_elf_headers() gets some error and
>> returns a negetive value, the elfcorehdr_addr should be released with
>> elfcorehdr_free().
>
>This is exceedingly minor - a single memory leak per boot, under very
>rare circumstances.
>
>
>With every patch I merge I consider -stable.  Often I'll discuss the
>desirability of a backport with the author and with reviewers.  Every
>single patch.  And then some damn script comes along and overrides that
>quite careful decision.  argh.
>
>Can we please do something like
>
>	if (akpm && !cc:stable)
>		dont_backport()

Yup, I already had it set for 'akpm && mm/ && !cc:stable', happy to
remove the 'mm/' restriction if you're doing the same for the rest of
the patches you review.

>And even go further - if your script thinks it might be something we
>should backport and if it didn't have cc:stable then contact the
>author, reviewers and committers and ask them to reconsider before we
>go and backport it.  This approach will have the advantage of training
>people to consider the backport more consistently.

This is what this mail is all about: I haven't queued up the patch yet,
it gives folks week+ to review, and all it takes is a simple "no" for me
to drop it.

>I'd (still) like to have a new patch tag like Not-For-Stable: or
>cc:not-stable or something to tell your scripts "yes, we thought about
>it and we decided no".

No objections on my part.

-- 
Thanks,
Sasha