Re: [dm-devel] [git pull] device mapper changes for 6.1

From: Mikulas Patocka <mpatocka@redhat.com>
To: Christoph Hellwig <hch@infradead.org>
Cc: Mike Snitzer <snitzer@redhat.com>,
	Jilin Yuan <yuanjilin@cdjrlc.com>,
	Nikos Tsironis <ntsironis@arrikto.com>,
	Shaomin Deng <dengshaomin@cdjrlc.com>,
	Mike Snitzer <snitzer@kernel.org>,
	Nathan Huckleberry <nhuck@google.com>,
	linux-block@vger.kernel.org, dm-devel@redhat.com,
	Genjian Zhang <zhanggenjian@kylinos.cn>,
	Linus Torvalds <torvalds@linux-foundation.org>,
	Milan Broz <gmazyland@gmail.com>,
	Alasdair G Kergon <agk@redhat.com>,
	Jiangshan Yi <yijiangshan@kylinos.cn>
Subject: Re: [dm-devel] [git pull] device mapper changes for 6.1
Date: Thu, 20 Oct 2022 07:44:14 -0400 (EDT)	[thread overview]
Message-ID: <alpine.LRH.2.21.2210200731150.25125@file01.intranet.prod.int.rdu2.redhat.com> (raw)
In-Reply-To: <Y1EDxZvjEi47iWUN@infradead.org>

On Thu, 20 Oct 2022, Christoph Hellwig wrote:

> On Tue, Oct 18, 2022 at 02:48:01PM -0400, Mike Snitzer wrote:
> > > That really does not sound like a good idea at all.  And it does not
> > > seem to have any MM or core maintainer signoffs.
> > 
> > Sorry, I should've solicited proper review more loudly.
> > 
> > But if you have a specific concern with how DM is looking to use
> > is_vmalloc_or_module_addr() please say what that is.
> 
> If I understand the patch correct it tries to use it to detect if
> a string is a string global.  Besides being nasty API abuse I can't
> see how this would even work if DM is built in.

It works for built-in DM.

You have "kfree_const" that detects if the string points to to .rodata 
with "is_kernel_rodata". Unfortunatelly, is_kernel_rodata doesn't detect 
if the string points to some modules's rodata, so we need an extra check 
for that.

So, the logic is:
if (!is_vmalloc_or_module_addr(ptr) && !is_kernel_rodata(ptr)) kfree(ptr);

I thought about modifying is_kernel_rodata to detect module's rodata as 
well, but it wouldn't work because kstrdup_const uses it and there would 
be crash possibility:
ptr = kstrdup_const(modules_rodata); unload_module(); use "ptr";

Mikulas