[PATCH] thunderbolt: debugfs: Replace get_zeroed_page() with kzalloc()

[PATCH] thunderbolt: debugfs: Replace get_zeroed_page() with kzalloc()

[Mahad Ibrahim]

validate_and_copy_from_user() allocates a page to store data from
userspace via get_zeroed_page(), and then returns it as a buffer.
Neither the function itself nor its callers require struct page access.

This buffer can easily be allocated with kzalloc() as there is nothing
special about it that requires going through the page allocator.
kzalloc(), which internally reduces to kmalloc() with __GFP_ZERO,
provides a better API and kfree() does not need to know the size of the
freed object. Additionally it removes the casts of (void *) and
(unsigned long) which only obfuscate the code.

Replace get_zeroed_page() with kzalloc() and free_page() with kfree().

Compile-tested only; no USB4 hardware available.

Link: https://lore.kernel.org/all/635405e4-9423-4a25-a6e7-e03c8ea0bcbe@redhat.com/
Signed-off-by: Mahad Ibrahim <mahad.ibrahim.dev@gmail.com>
---
 drivers/thunderbolt/debugfs.c | 28 ++++++++++++++--------------
 1 file changed, 14 insertions(+), 14 deletions(-)

diff --git a/drivers/thunderbolt/debugfs.c b/drivers/thunderbolt/debugfs.c
index f5cf0e177f40..6e9080e7bcec 100644
--- a/drivers/thunderbolt/debugfs.c
+++ b/drivers/thunderbolt/debugfs.c
@@ -136,13 +136,13 @@ static void *validate_and_copy_from_user(const void __user *user_buf,
 	if (!access_ok(user_buf, *count))
 		return ERR_PTR(-EFAULT);
 
-	buf = (void *)get_zeroed_page(GFP_KERNEL);
+	buf = kzalloc(PAGE_SIZE, GFP_KERNEL);
 	if (!buf)
 		return ERR_PTR(-ENOMEM);
 
 	nbytes = min_t(size_t, *count, PAGE_SIZE);
 	if (copy_from_user(buf, user_buf, nbytes)) {
-		free_page((unsigned long)buf);
+		kfree(buf);
 		return ERR_PTR(-EFAULT);
 	}
 
@@ -265,7 +265,7 @@ static ssize_t regs_write(struct tb_switch *sw, struct tb_port *port,
 out:
 	pm_runtime_mark_last_busy(&sw->dev);
 	pm_runtime_put_autosuspend(&sw->dev);
-	free_page((unsigned long)buf);
+	kfree(buf);
 
 	return ret < 0 ? ret : count;
 }
@@ -406,7 +406,7 @@ static ssize_t port_sb_regs_write(struct file *file, const char __user *user_buf
 out:
 	pm_runtime_mark_last_busy(&sw->dev);
 	pm_runtime_put_autosuspend(&sw->dev);
-	free_page((unsigned long)buf);
+	kfree(buf);
 
 	return ret < 0 ? ret : count;
 }
@@ -439,7 +439,7 @@ static ssize_t retimer_sb_regs_write(struct file *file,
 out:
 	pm_runtime_mark_last_busy(&rt->dev);
 	pm_runtime_put_autosuspend(&rt->dev);
-	free_page((unsigned long)buf);
+	kfree(buf);
 
 	return ret < 0 ? ret : count;
 }
@@ -652,7 +652,7 @@ margining_ber_level_write(struct file *file, const char __user *user_buf,
 	margining->ber_level = val;
 
 out_free:
-	free_page((unsigned long)buf);
+	kfree(buf);
 out_unlock:
 	mutex_unlock(&tb->lock);
 
@@ -829,7 +829,7 @@ margining_lanes_write(struct file *file, const char __user *user_buf,
 		}
 	}
 
-	free_page((unsigned long)buf);
+	kfree(buf);
 
 	if (lane == -1)
 		return -EINVAL;
@@ -958,7 +958,7 @@ margining_error_counter_write(struct file *file, const char __user *user_buf,
 	else
 		goto err_free;
 
-	free_page((unsigned long)buf);
+	kfree(buf);
 
 	scoped_cond_guard(mutex_intr, return -ERESTARTSYS, &tb->lock) {
 		if (!margining->software)
@@ -970,7 +970,7 @@ margining_error_counter_write(struct file *file, const char __user *user_buf,
 	return count;
 
 err_free:
-	free_page((unsigned long)buf);
+	kfree(buf);
 	return -EINVAL;
 }
 
@@ -1116,7 +1116,7 @@ static ssize_t margining_mode_write(struct file *file,
 	mutex_unlock(&tb->lock);
 
 out_free:
-	free_page((unsigned long)buf);
+	kfree(buf);
 	return ret ? ret : count;
 }
 
@@ -1503,7 +1503,7 @@ static ssize_t margining_test_write(struct file *file,
 	mutex_unlock(&tb->lock);
 
 out_free:
-	free_page((unsigned long)buf);
+	kfree(buf);
 	return ret ? ret : count;
 }
 
@@ -1569,7 +1569,7 @@ static ssize_t margining_margin_write(struct file *file,
 	mutex_unlock(&tb->lock);
 
 out_free:
-	free_page((unsigned long)buf);
+	kfree(buf);
 	return ret ? ret : count;
 }
 
@@ -1624,7 +1624,7 @@ static ssize_t margining_eye_write(struct file *file,
 			ret = -EINVAL;
 	}
 
-	free_page((unsigned long)buf);
+	kfree(buf);
 	return ret ? ret : count;
 }
 
@@ -1934,7 +1934,7 @@ static ssize_t counters_write(struct file *file, const char __user *user_buf,
 out:
 	pm_runtime_mark_last_busy(&sw->dev);
 	pm_runtime_put_autosuspend(&sw->dev);
-	free_page((unsigned long)buf);
+	kfree(buf);
 
 	return ret < 0 ? ret : count;
 }
-- 
2.54.0

Re: [PATCH] thunderbolt: debugfs: Replace get_zeroed_page() with kzalloc()

[Mahad Ibrahim]

Some context I meant to include below the --- and accidentally forgot:

Mike Rapoport is running a broader effort to move allocations that do
not need struct page access off the page allocator. He does not seem to
be maintaining a tree for it, so landing this via your own tree would
be appreciated.

The buffer is kept at PAGE_SIZE to preserve the existing truncation
behaviour. If you would prefer it sized to the actual write instead,
I can send a v2.

Best regards,
Mahad Ibrahim

Re: [PATCH] thunderbolt: debugfs: Replace get_zeroed_page() with kzalloc()

[Mike Rapoport]

On Sat, Aug 08, 2026 at 10:38:29PM +0000, Mahad Ibrahim wrote:
> validate_and_copy_from_user() allocates a page to store data from
> userspace via get_zeroed_page(), and then returns it as a buffer.
> Neither the function itself nor its callers require struct page access.
> 
> This buffer can easily be allocated with kzalloc() as there is nothing
> special about it that requires going through the page allocator.
> kzalloc(), which internally reduces to kmalloc() with __GFP_ZERO,
> provides a better API and kfree() does not need to know the size of the
> freed object. Additionally it removes the casts of (void *) and
> (unsigned long) which only obfuscate the code.
> 
> Replace get_zeroed_page() with kzalloc() and free_page() with kfree().
> 
> Compile-tested only; no USB4 hardware available.
> 
> Link: https://lore.kernel.org/all/635405e4-9423-4a25-a6e7-e03c8ea0bcbe@redhat.com/
> Signed-off-by: Mahad Ibrahim <mahad.ibrahim.dev@gmail.com>

Acked-by: Mike Rapoport (Microsoft) <rppt@kernel.org>

> ---
>  drivers/thunderbolt/debugfs.c | 28 ++++++++++++++--------------
>  1 file changed, 14 insertions(+), 14 deletions(-)

-- 
Sincerely yours,
Mike.

Re: [PATCH] thunderbolt: debugfs: Replace get_zeroed_page() with kzalloc()

[Mika Westerberg]

Hi,

On Sat, Aug 08, 2026 at 10:38:29PM +0000, Mahad Ibrahim wrote:
> validate_and_copy_from_user() allocates a page to store data from
> userspace via get_zeroed_page(), and then returns it as a buffer.
> Neither the function itself nor its callers require struct page access.
> 
> This buffer can easily be allocated with kzalloc() as there is nothing
> special about it that requires going through the page allocator.
> kzalloc(), which internally reduces to kmalloc() with __GFP_ZERO,
> provides a better API and kfree() does not need to know the size of the
> freed object. Additionally it removes the casts of (void *) and
> (unsigned long) which only obfuscate the code.
> 
> Replace get_zeroed_page() with kzalloc() and free_page() with kfree().
> 
> Compile-tested only; no USB4 hardware available.

I tested this on Intel HW and works fine.

> Link: https://lore.kernel.org/all/635405e4-9423-4a25-a6e7-e03c8ea0bcbe@redhat.com/
> Signed-off-by: Mahad Ibrahim <mahad.ibrahim.dev@gmail.com>

Applied to thunderbolt.git/next with Mike's ack. Thanks!