[PATCH 0/4] (k)vrealloc (__GFP_ZERO) fixes

[PATCH 0/4] (k)vrealloc (__GFP_ZERO) fixes

[Danilo Krummrich]

Fixups for [1], as discussed.

[1] https://lore.kernel.org/linux-mm/20240722163111.4766-1-dakr@kernel.org/

Danilo Krummrich (4):
  mm: kvrealloc: disable KASAN when switching to vmalloc
  mm: vrealloc: consider spare memory for __GFP_ZERO
  mm: vrealloc: properly document __GFP_ZERO behavior
  mm: kvrealloc: properly document __GFP_ZERO behavior

 mm/util.c    | 16 ++++++++++++----
 mm/vmalloc.c | 24 ++++++++++++++++--------
 2 files changed, 28 insertions(+), 12 deletions(-)


base-commit: 4152c2f7b8af8c270686a4aabda302ec22b0e099
-- 
2.45.2

[PATCH 1/4] mm: kvrealloc: disable KASAN when switching to vmalloc

[Danilo Krummrich]

Disable KASAN accessibility checks when switching from a kmalloc buffer
to a vmalloc buffer.

Fixes: 923a26b4c679 ("mm: kvmalloc: align kvrealloc() with krealloc()")
Signed-off-by: Danilo Krummrich <dakr@kernel.org>
---
 mm/util.c | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/mm/util.c b/mm/util.c
index 29ae93f6344f..bfb2d69b6434 100644
--- a/mm/util.c
+++ b/mm/util.c
@@ -753,7 +753,10 @@ void *kvrealloc_noprof(const void *p, size_t size, gfp_t flags)
 
 		if (p) {
 			/* We already know that `p` is not a vmalloc address. */
-			memcpy(n, p, ksize(p));
+			kasan_disable_current();
+			memcpy(n, kasan_reset_tag(p), ksize(p));
+			kasan_enable_current();
+
 			kfree(p);
 		}
 	}
-- 
2.45.2

Re: [PATCH 1/4] mm: kvrealloc: disable KASAN when switching to vmalloc

[Andrew Morton]

On Tue, 30 Jul 2024 20:49:41 +0200 Danilo Krummrich <dakr@kernel.org> wrote:

> Disable KASAN accessibility checks when switching from a kmalloc buffer
> to a vmalloc buffer.

This text tells us "what", which was utterly evident from a cursory
read of the code.

Please tell us the "why".  Completely.

> Fixes: 923a26b4c679 ("mm: kvmalloc: align kvrealloc() with krealloc()")

For those who are following along, this patch in mm-unstable so this
patch will be squished into the above.

Re: [PATCH 1/4] mm: kvrealloc: disable KASAN when switching to vmalloc

[Danilo Krummrich]

On Tue, Jul 30, 2024 at 02:15:08PM -0700, Andrew Morton wrote:
> On Tue, 30 Jul 2024 20:49:41 +0200 Danilo Krummrich <dakr@kernel.org> wrote:
> 
> > Disable KASAN accessibility checks when switching from a kmalloc buffer
> > to a vmalloc buffer.
> 
> This text tells us "what", which was utterly evident from a cursory
> read of the code.
> 
> Please tell us the "why".  Completely.

As mentioned in the other reply. Since this is a fixup series for stuff that's
in mm-unstable already, and hence gets squashed in later on, I treated this more
like the list of changes one would also append when sending the next version of
a series.

If you expect such fixup commits to have the same detail than regular ones, I'll
surely add those details and resend. Please let me know.

> 
> > Fixes: 923a26b4c679 ("mm: kvmalloc: align kvrealloc() with krealloc()")
> 
> For those who are following along, this patch in mm-unstable so this
> patch will be squished into the above.
> 

[PATCH 2/4] mm: vrealloc: consider spare memory for __GFP_ZERO

[Danilo Krummrich]

Zero spare memory when shrinking a buffer with __GFP_ZERO.

Fixes: 1f39ee9615a8 ("mm: vmalloc: implement vrealloc()")
Signed-off-by: Danilo Krummrich <dakr@kernel.org>
---
 mm/vmalloc.c | 13 ++++++++-----
 1 file changed, 8 insertions(+), 5 deletions(-)

diff --git a/mm/vmalloc.c b/mm/vmalloc.c
index 2a6d4ce57b73..6a2fef6378e4 100644
--- a/mm/vmalloc.c
+++ b/mm/vmalloc.c
@@ -4077,12 +4077,15 @@ void *vrealloc_noprof(const void *p, size_t size, gfp_t flags)
 		old_size = get_vm_area_size(vm);
 	}
 
+	/*
+	 * TODO: Shrink the vm_area, i.e. unmap and free unused pages. What
+	 * would be a good heuristic for when to shrink the vm_area?
+	 */
 	if (size <= old_size) {
-		/*
-		 * TODO: Shrink the vm_area, i.e. unmap and free unused pages.
-		 * What would be a good heuristic for when to shrink the
-		 * vm_area?
-		 */
+		/* Zero out spare memory. */
+		if (want_init_on_alloc(flags))
+			memset((void *)p + size, 0, old_size - size);
+
 		return (void *)p;
 	}
 
-- 
2.45.2

Re: [PATCH 2/4] mm: vrealloc: consider spare memory for __GFP_ZERO

[Andrew Morton]

On Tue, 30 Jul 2024 20:49:42 +0200 Danilo Krummrich <dakr@kernel.org> wrote:

> Zero spare memory when shrinking a buffer with __GFP_ZERO.

Again, why?

[PATCH 3/4] mm: vrealloc: properly document __GFP_ZERO behavior

[Danilo Krummrich]

Properly document that if __GFP_ZERO logic is requested, callers must
ensure that, starting with the initial memory allocation, every
subsequent call to this API for the same memory allocation is flagged
with __GFP_ZERO. Otherwise, it is possible that __GFP_ZERO is not fully
honored by this API.

Signed-off-by: Danilo Krummrich <dakr@kernel.org>
---
 mm/vmalloc.c | 11 ++++++++---
 1 file changed, 8 insertions(+), 3 deletions(-)

diff --git a/mm/vmalloc.c b/mm/vmalloc.c
index 6a2fef6378e4..48cc10dd06c0 100644
--- a/mm/vmalloc.c
+++ b/mm/vmalloc.c
@@ -4043,12 +4043,17 @@ EXPORT_SYMBOL(vzalloc_node_noprof);
  * @size: the size to reallocate
  * @flags: the flags for the page level allocator
  *
- * The contents of the object pointed to are preserved up to the lesser of the
- * new and old size (__GFP_ZERO flag is effectively ignored).
- *
  * If @p is %NULL, vrealloc() behaves exactly like vmalloc(). If @size is 0 and
  * @p is not a %NULL pointer, the object pointed to is freed.
  *
+ * If __GFP_ZERO logic is requested, callers must ensure that, starting with the
+ * initial memory allocation, every subsequent call to this API for the same
+ * memory allocation is flagged with __GFP_ZERO. Otherwise, it is possible that
+ * __GFP_ZERO is not fully honored by this API.
+ *
+ * In any case, the contents of the object pointed to are preserved up to the
+ * lesser of the new and old sizes.
+ *
  * This function must not be called concurrently with itself or vfree() for the
  * same memory allocation.
  *
-- 
2.45.2

Re: [PATCH 3/4] mm: vrealloc: properly document __GFP_ZERO behavior

[Andrew Morton]

On Tue, 30 Jul 2024 20:49:43 +0200 Danilo Krummrich <dakr@kernel.org> wrote:

> Properly document that if __GFP_ZERO logic is requested, callers must
> ensure that, starting with the initial memory allocation, every
> subsequent call to this API for the same memory allocation is flagged
> with __GFP_ZERO. Otherwise, it is possible that __GFP_ZERO is not fully
> honored by this API.

I appear to have just seen this, in a separate mailing.

Please, slow down.  We have two months.  Await reviewer feedback, spend
time over those changelogs, value clarity and accuracy and completeness
over hastiness.  The only reason for rushing things is if a patch is
disrupting ongoing testing of the linux-next tree.

Re: [PATCH 3/4] mm: vrealloc: properly document __GFP_ZERO behavior

[Danilo Krummrich]

On Tue, Jul 30, 2024 at 02:19:53PM -0700, Andrew Morton wrote:
> On Tue, 30 Jul 2024 20:49:43 +0200 Danilo Krummrich <dakr@kernel.org> wrote:
> 
> > Properly document that if __GFP_ZERO logic is requested, callers must
> > ensure that, starting with the initial memory allocation, every
> > subsequent call to this API for the same memory allocation is flagged
> > with __GFP_ZERO. Otherwise, it is possible that __GFP_ZERO is not fully
> > honored by this API.
> 
> I appear to have just seen this, in a separate mailing.

What you have seen in a separate mail is a similar patch for krealloc() [1].
This one is a fixup for vrealloc() from a previous submission you've applied to
mm-unstable.

> 
> Please, slow down.  We have two months.  Await reviewer feedback, spend
> time over those changelogs, value clarity and accuracy and completeness
> over hastiness.  The only reason for rushing things is if a patch is
> disrupting ongoing testing of the linux-next tree.

There was a discussion in [2], which lead to this fixup series.

In terms of changelogs this series is indeed a bit "lax", since I have
recognized that you queue up fixup patches for changes that did already land in
mm-unstable to be squashed into the original commits later on.

[1] https://lore.kernel.org/linux-mm/20240730194214.31483-1-dakr@kernel.org/
[2] https://lore.kernel.org/linux-mm/20240722163111.4766-1-dakr@kernel.org/T/#m065a7f875b44dc945dd535c2b7168c3d77a98993

Re: [PATCH 3/4] mm: vrealloc: properly document __GFP_ZERO behavior

[Vlastimil Babka]

On 7/31/24 12:43 AM, Danilo Krummrich wrote:
> On Tue, Jul 30, 2024 at 02:19:53PM -0700, Andrew Morton wrote:
>> On Tue, 30 Jul 2024 20:49:43 +0200 Danilo Krummrich <dakr@kernel.org> wrote:
>> 
>> > Properly document that if __GFP_ZERO logic is requested, callers must
>> > ensure that, starting with the initial memory allocation, every
>> > subsequent call to this API for the same memory allocation is flagged
>> > with __GFP_ZERO. Otherwise, it is possible that __GFP_ZERO is not fully
>> > honored by this API.
>> 
>> I appear to have just seen this, in a separate mailing.
> 
> What you have seen in a separate mail is a similar patch for krealloc() [1].
> This one is a fixup for vrealloc() from a previous submission you've applied to
> mm-unstable.
> 
>> 
>> Please, slow down.  We have two months.  Await reviewer feedback, spend
>> time over those changelogs, value clarity and accuracy and completeness
>> over hastiness.  The only reason for rushing things is if a patch is
>> disrupting ongoing testing of the linux-next tree.
> 
> There was a discussion in [2], which lead to this fixup series.
> 
> In terms of changelogs this series is indeed a bit "lax", since I have
> recognized that you queue up fixup patches for changes that did already land in
> mm-unstable to be squashed into the original commits later on.

Some of the changes in the fixups would however ideally result in udpdates
to the original changelogs in addition to squashing code. Also with 4 fixups
to 2 original patches it might be IMHO better to squash on your side and
resend as a full replacement. Perhaps also together with the other 2 patches
about __GFP_ZERO for krealloc in a single series. As Andrew mentioned we are
early in the rc phase to afford this.

> [1] https://lore.kernel.org/linux-mm/20240730194214.31483-1-dakr@kernel.org/
> [2] https://lore.kernel.org/linux-mm/20240722163111.4766-1-dakr@kernel.org/T/#m065a7f875b44dc945dd535c2b7168c3d77a98993

Re: [PATCH 3/4] mm: vrealloc: properly document __GFP_ZERO behavior

[Danilo Krummrich]

On Wed, Jul 31, 2024 at 04:43:39PM +0200, Vlastimil Babka wrote:
> On 7/31/24 12:43 AM, Danilo Krummrich wrote:
> > On Tue, Jul 30, 2024 at 02:19:53PM -0700, Andrew Morton wrote:
> >> On Tue, 30 Jul 2024 20:49:43 +0200 Danilo Krummrich <dakr@kernel.org> wrote:
> >> 
> >> > Properly document that if __GFP_ZERO logic is requested, callers must
> >> > ensure that, starting with the initial memory allocation, every
> >> > subsequent call to this API for the same memory allocation is flagged
> >> > with __GFP_ZERO. Otherwise, it is possible that __GFP_ZERO is not fully
> >> > honored by this API.
> >> 
> >> I appear to have just seen this, in a separate mailing.
> > 
> > What you have seen in a separate mail is a similar patch for krealloc() [1].
> > This one is a fixup for vrealloc() from a previous submission you've applied to
> > mm-unstable.
> > 
> >> 
> >> Please, slow down.  We have two months.  Await reviewer feedback, spend
> >> time over those changelogs, value clarity and accuracy and completeness
> >> over hastiness.  The only reason for rushing things is if a patch is
> >> disrupting ongoing testing of the linux-next tree.
> > 
> > There was a discussion in [2], which lead to this fixup series.
> > 
> > In terms of changelogs this series is indeed a bit "lax", since I have
> > recognized that you queue up fixup patches for changes that did already land in
> > mm-unstable to be squashed into the original commits later on.
> 
> Some of the changes in the fixups would however ideally result in udpdates
> to the original changelogs in addition to squashing code. Also with 4 fixups
> to 2 original patches it might be IMHO better to squash on your side and
> resend as a full replacement. Perhaps also together with the other 2 patches
> about __GFP_ZERO for krealloc in a single series. As Andrew mentioned we are
> early in the rc phase to afford this.

(JFYI, Andrew applied the fixups meanwhile.)

I also don't think that they lead to updates of the commit messages.

But yes, we can proceed with:

(1) leave [1] as it is (with the fixups applied to mm-unstable already) and send
    v2 of [2]
(2) send a v3 for [1] that also includes [2]
(3) send a v3 of [1] and a separate v2 for [2]

Just let me know what you prefer. I'm fine with either of those. :)

[1] https://lore.kernel.org/linux-mm/20240722163111.4766-1-dakr@kernel.org/
[2] https://lore.kernel.org/linux-mm/20240730194214.31483-1-dakr@kernel.org/

[PATCH 4/4] mm: kvrealloc: properly document __GFP_ZERO behavior

[Danilo Krummrich]

Properly document that if __GFP_ZERO logic is requested, callers must
ensure that, starting with the initial memory allocation, every
subsequent call to this API for the same memory allocation is flagged
with __GFP_ZERO. Otherwise, it is possible that __GFP_ZERO is not fully
honored by this API.

Signed-off-by: Danilo Krummrich <dakr@kernel.org>
---
 mm/util.c | 11 ++++++++---
 1 file changed, 8 insertions(+), 3 deletions(-)

diff --git a/mm/util.c b/mm/util.c
index bfb2d69b6434..f899b0f984a0 100644
--- a/mm/util.c
+++ b/mm/util.c
@@ -726,12 +726,17 @@ EXPORT_SYMBOL(kvfree_sensitive);
  * @size: the size to reallocate
  * @flags: the flags for the page level allocator
  *
- * The contents of the object pointed to are preserved up to the lesser of the
- * new and old size (__GFP_ZERO flag is effectively ignored).
- *
  * If @p is %NULL, kvrealloc() behaves exactly like kvmalloc(). If @size is 0
  * and @p is not a %NULL pointer, the object pointed to is freed.
  *
+ * If __GFP_ZERO logic is requested, callers must ensure that, starting with the
+ * initial memory allocation, every subsequent call to this API for the same
+ * memory allocation is flagged with __GFP_ZERO. Otherwise, it is possible that
+ * __GFP_ZERO is not fully honored by this API.
+ *
+ * In any case, the contents of the object pointed to are preserved up to the
+ * lesser of the new and old sizes.
+ *
  * This function must not be called concurrently with itself or kvfree() for the
  * same memory allocation.
  *
-- 
2.45.2