* [PATCH] lkdtm: use kmalloc() instead of __get_free_page
@ 2026-07-22 23:02 Mahad Ibrahim
2026-07-22 23:42 ` Mahad Ibrahim
2026-07-23 8:45 ` David Laight
0 siblings, 2 replies; 5+ messages in thread
From: Mahad Ibrahim @ 2026-07-22 23:02 UTC (permalink / raw)
To: Kees Cook
Cc: Arnd Bergmann, Greg Kroah-Hartman, Mike Rapoport, linux-mm,
linux-kernel, Mahad Ibrahim
lkdtm_debugfs_entry and direct_entry use __get_free_page to allocate a
temporary buffer, perform copy_from_user to get the crashtype name,
strim() to strip whitespace and find_crashtype to find the corresponding
crashtype that is being requested.
The lkdtm_debugfs_read uses __get_free_page to allocate a temporary
buffer to store all the available crashtypes, and then copy it to
userspace.
The buffers that are allocated can be allocated with kmalloc as there is
nothing special that requires a struct page, or the page allocator.
kmalloc() additionally provides a better API that doesn't require ugly
casts which obfuscate the code and kfree does not need to know the size
of the freed object.
Replace use of __get_free_page() with kmalloc().
Signed-off-by: Mahad Ibrahim <mahad.ibrahim.dev@gmail.com>
---
drivers/misc/lkdtm/core.c | 16 ++++++++--------
1 file changed, 8 insertions(+), 8 deletions(-)
diff --git a/drivers/misc/lkdtm/core.c b/drivers/misc/lkdtm/core.c
index ededa32d6744..01bebcb33bd4 100644
--- a/drivers/misc/lkdtm/core.c
+++ b/drivers/misc/lkdtm/core.c
@@ -236,11 +236,11 @@ static ssize_t lkdtm_debugfs_entry(struct file *f,
if (count >= PAGE_SIZE)
return -EINVAL;
- buf = (char *)__get_free_page(GFP_KERNEL);
+ buf = kmalloc(PAGE_SIZE, GFP_KERNEL);
if (!buf)
return -ENOMEM;
if (copy_from_user(buf, user_buf, count)) {
- free_page((unsigned long) buf);
+ kfree(buf);
return -EFAULT;
}
/* NULL-terminate and remove enter */
@@ -248,7 +248,7 @@ static ssize_t lkdtm_debugfs_entry(struct file *f,
strim(buf);
crashtype = find_crashtype(buf);
- free_page((unsigned long)buf);
+ kfree(buf);
if (!crashtype)
return -EINVAL;
@@ -271,7 +271,7 @@ static ssize_t lkdtm_debugfs_read(struct file *f, char __user *user_buf,
ssize_t out;
char *buf;
- buf = (char *)__get_free_page(GFP_KERNEL);
+ buf = kmalloc(PAGE_SIZE, GFP_KERNEL);
if (buf == NULL)
return -ENOMEM;
@@ -290,7 +290,7 @@ static ssize_t lkdtm_debugfs_read(struct file *f, char __user *user_buf,
out = simple_read_from_buffer(user_buf, count, off,
buf, n);
- free_page((unsigned long) buf);
+ kfree(buf);
return out;
}
@@ -313,11 +313,11 @@ static ssize_t direct_entry(struct file *f, const char __user *user_buf,
if (count < 1)
return -EINVAL;
- buf = (char *)__get_free_page(GFP_KERNEL);
+ buf = kmalloc(PAGE_SIZE, GFP_KERNEL);
if (!buf)
return -ENOMEM;
if (copy_from_user(buf, user_buf, count)) {
- free_page((unsigned long) buf);
+ kfree(buf);
return -EFAULT;
}
/* NULL-terminate and remove enter */
@@ -325,7 +325,7 @@ static ssize_t direct_entry(struct file *f, const char __user *user_buf,
strim(buf);
crashtype = find_crashtype(buf);
- free_page((unsigned long) buf);
+ kfree(buf);
if (!crashtype)
return -EINVAL;
--
2.54.0
^ permalink raw reply related [flat|nested] 5+ messages in thread* Re: [PATCH] lkdtm: use kmalloc() instead of __get_free_page
2026-07-22 23:02 [PATCH] lkdtm: use kmalloc() instead of __get_free_page Mahad Ibrahim
@ 2026-07-22 23:42 ` Mahad Ibrahim
2026-07-23 3:47 ` Kees Cook
2026-07-23 8:45 ` David Laight
1 sibling, 1 reply; 5+ messages in thread
From: Mahad Ibrahim @ 2026-07-22 23:42 UTC (permalink / raw)
To: Mahad Ibrahim, Kees Cook
Cc: Arnd Bergmann, Greg Kroah-Hartman, Mike Rapoport, linux-mm,
linux-kernel
For context, this is part of the broader __get_free_page() -> kmalloc()
transition Mike Rapoport is performing. I noticed it on the mailing
lists and found this site which wasn't covered.
Some background:
[1] https://lore.kernel.org/all/CA+55aFwp4iy4rtX2gE2WjBGFL=NxMVnoFeHqYa2j1dYOMMGqxg@mail.gmail.com/
[2] https://lore.kernel.org/linux-scsi/20260704-b4-scsi-v2-0-7d2d21a810de@kernel.org/T/#t
I apologize for this messy reply. I thought I had added the context, but
I didn't.
Best regards,
Mahad Ibrahim
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] lkdtm: use kmalloc() instead of __get_free_page
2026-07-22 23:42 ` Mahad Ibrahim
@ 2026-07-23 3:47 ` Kees Cook
0 siblings, 0 replies; 5+ messages in thread
From: Kees Cook @ 2026-07-23 3:47 UTC (permalink / raw)
To: Mahad Ibrahim
Cc: Arnd Bergmann, Greg Kroah-Hartman, Mike Rapoport, linux-mm,
linux-kernel
On Wed, Jul 22, 2026 at 11:42:25PM +0000, Mahad Ibrahim wrote:
> For context, this is part of the broader __get_free_page() -> kmalloc()
> transition Mike Rapoport is performing. I noticed it on the mailing
> lists and found this site which wasn't covered.
>
> Some background:
>
> [1] https://lore.kernel.org/all/CA+55aFwp4iy4rtX2gE2WjBGFL=NxMVnoFeHqYa2j1dYOMMGqxg@mail.gmail.com/
> [2] https://lore.kernel.org/linux-scsi/20260704-b4-scsi-v2-0-7d2d21a810de@kernel.org/T/#t
>
> I apologize for this messy reply. I thought I had added the context, but
> I didn't.
Do you want this to land via my tree or via the series Mike is doing?
--
Kees Cook
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] lkdtm: use kmalloc() instead of __get_free_page
2026-07-22 23:02 [PATCH] lkdtm: use kmalloc() instead of __get_free_page Mahad Ibrahim
2026-07-22 23:42 ` Mahad Ibrahim
@ 2026-07-23 8:45 ` David Laight
2026-07-23 9:00 ` Arnd Bergmann
1 sibling, 1 reply; 5+ messages in thread
From: David Laight @ 2026-07-23 8:45 UTC (permalink / raw)
To: Mahad Ibrahim
Cc: Kees Cook, Arnd Bergmann, Greg Kroah-Hartman, Mike Rapoport,
linux-mm, linux-kernel
On Wed, 22 Jul 2026 23:02:46 +0000
Mahad Ibrahim <mahad.ibrahim.dev@gmail.com> wrote:
> lkdtm_debugfs_entry and direct_entry use __get_free_page to allocate a
> temporary buffer, perform copy_from_user to get the crashtype name,
> strim() to strip whitespace and find_crashtype to find the corresponding
> crashtype that is being requested.
>
> The lkdtm_debugfs_read uses __get_free_page to allocate a temporary
> buffer to store all the available crashtypes, and then copy it to
> userspace.
>
> The buffers that are allocated can be allocated with kmalloc as there is
> nothing special that requires a struct page, or the page allocator.
>
> kmalloc() additionally provides a better API that doesn't require ugly
> casts which obfuscate the code and kfree does not need to know the size
> of the freed object.
>
> Replace use of __get_free_page() with kmalloc().
None of those buffers need to be PAGE_SIZE.
Even the sanity limit for overlong requests could be 4k.
The longest 'crashtype->name' is probably about 32 characters, so could
probably go on stack.
And can't this code use the sysfs/kernfs wrappers?
David
>
> Signed-off-by: Mahad Ibrahim <mahad.ibrahim.dev@gmail.com>
> ---
> drivers/misc/lkdtm/core.c | 16 ++++++++--------
> 1 file changed, 8 insertions(+), 8 deletions(-)
>
> diff --git a/drivers/misc/lkdtm/core.c b/drivers/misc/lkdtm/core.c
> index ededa32d6744..01bebcb33bd4 100644
> --- a/drivers/misc/lkdtm/core.c
> +++ b/drivers/misc/lkdtm/core.c
> @@ -236,11 +236,11 @@ static ssize_t lkdtm_debugfs_entry(struct file *f,
> if (count >= PAGE_SIZE)
> return -EINVAL;
>
> - buf = (char *)__get_free_page(GFP_KERNEL);
> + buf = kmalloc(PAGE_SIZE, GFP_KERNEL);
> if (!buf)
> return -ENOMEM;
> if (copy_from_user(buf, user_buf, count)) {
> - free_page((unsigned long) buf);
> + kfree(buf);
> return -EFAULT;
> }
> /* NULL-terminate and remove enter */
> @@ -248,7 +248,7 @@ static ssize_t lkdtm_debugfs_entry(struct file *f,
> strim(buf);
>
> crashtype = find_crashtype(buf);
> - free_page((unsigned long)buf);
> + kfree(buf);
>
> if (!crashtype)
> return -EINVAL;
> @@ -271,7 +271,7 @@ static ssize_t lkdtm_debugfs_read(struct file *f, char __user *user_buf,
> ssize_t out;
> char *buf;
>
> - buf = (char *)__get_free_page(GFP_KERNEL);
> + buf = kmalloc(PAGE_SIZE, GFP_KERNEL);
> if (buf == NULL)
> return -ENOMEM;
>
> @@ -290,7 +290,7 @@ static ssize_t lkdtm_debugfs_read(struct file *f, char __user *user_buf,
>
> out = simple_read_from_buffer(user_buf, count, off,
> buf, n);
> - free_page((unsigned long) buf);
> + kfree(buf);
>
> return out;
> }
> @@ -313,11 +313,11 @@ static ssize_t direct_entry(struct file *f, const char __user *user_buf,
> if (count < 1)
> return -EINVAL;
>
> - buf = (char *)__get_free_page(GFP_KERNEL);
> + buf = kmalloc(PAGE_SIZE, GFP_KERNEL);
> if (!buf)
> return -ENOMEM;
> if (copy_from_user(buf, user_buf, count)) {
> - free_page((unsigned long) buf);
> + kfree(buf);
> return -EFAULT;
> }
> /* NULL-terminate and remove enter */
> @@ -325,7 +325,7 @@ static ssize_t direct_entry(struct file *f, const char __user *user_buf,
> strim(buf);
>
> crashtype = find_crashtype(buf);
> - free_page((unsigned long) buf);
> + kfree(buf);
> if (!crashtype)
> return -EINVAL;
>
^ permalink raw reply [flat|nested] 5+ messages in thread* Re: [PATCH] lkdtm: use kmalloc() instead of __get_free_page
2026-07-23 8:45 ` David Laight
@ 2026-07-23 9:00 ` Arnd Bergmann
0 siblings, 0 replies; 5+ messages in thread
From: Arnd Bergmann @ 2026-07-23 9:00 UTC (permalink / raw)
To: David Laight, Mahad Ibrahim
Cc: Kees Cook, Greg Kroah-Hartman, Mike Rapoport, linux-mm,
linux-kernel
On Thu, Jul 23, 2026, at 10:45, David Laight wrote:
> On Wed, 22 Jul 2026 23:02:46 +0000
> Mahad Ibrahim <mahad.ibrahim.dev@gmail.com> wrote:
>
>> lkdtm_debugfs_entry and direct_entry use __get_free_page to allocate a
>> temporary buffer, perform copy_from_user to get the crashtype name,
>> strim() to strip whitespace and find_crashtype to find the corresponding
>> crashtype that is being requested.
>>
>> The lkdtm_debugfs_read uses __get_free_page to allocate a temporary
>> buffer to store all the available crashtypes, and then copy it to
>> userspace.
>>
>> The buffers that are allocated can be allocated with kmalloc as there is
>> nothing special that requires a struct page, or the page allocator.
>>
>> kmalloc() additionally provides a better API that doesn't require ugly
>> casts which obfuscate the code and kfree does not need to know the size
>> of the freed object.
>>
>> Replace use of __get_free_page() with kmalloc().
>
> None of those buffers need to be PAGE_SIZE.
> Even the sanity limit for overlong requests could be 4k.
> The longest 'crashtype->name' is probably about 32 characters, so could
> probably go on stack.
It is a straightforward change with a very low risk of regression,
so I see nothing wrong with this version.
If we wanted to improve readability here, I would suggest
using strndup_user() to fold ten lines into one.
Arnd
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2026-07-23 9:00 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-07-22 23:02 [PATCH] lkdtm: use kmalloc() instead of __get_free_page Mahad Ibrahim
2026-07-22 23:42 ` Mahad Ibrahim
2026-07-23 3:47 ` Kees Cook
2026-07-23 8:45 ` David Laight
2026-07-23 9:00 ` Arnd Bergmann
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox