* [PATCH] proc: use kmalloc instead of __get_free_page() to alloc path buffer
@ 2022-01-23 10:08 Hao Lee
2022-01-23 13:55 ` Alexey Dobriyan
2022-01-25 14:03 ` [PATCH] proc: use kmalloc instead of __get_free_page() to alloc path buffer Torin Carey
0 siblings, 2 replies; 5+ messages in thread
From: Hao Lee @ 2022-01-23 10:08 UTC (permalink / raw)
To: akpm
Cc: christian.brauner, keescook, adobriyan, jamorris, linux-fsdevel,
linux-kernel, haolee.swjtu
It's not a standard approach that use __get_free_page() to alloc path
buffer directly. We'd better use kmalloc and PATH_MAX.
Signed-off-by: Hao Lee <haolee.swjtu@gmail.com>
---
fs/proc/base.c | 11 ++++++-----
1 file changed, 6 insertions(+), 5 deletions(-)
diff --git a/fs/proc/base.c b/fs/proc/base.c
index d654ce7150fd..74cfef87fe45 100644
--- a/fs/proc/base.c
+++ b/fs/proc/base.c
@@ -1764,25 +1764,26 @@ static const char *proc_pid_get_link(struct dentry *dentry,
static int do_proc_readlink(struct path *path, char __user *buffer, int buflen)
{
- char *tmp = (char *)__get_free_page(GFP_KERNEL);
+ char *buf = NULL;
char *pathname;
int len;
- if (!tmp)
+ buf = kmalloc(PATH_MAX, GFP_KERNEL);
+ if (!buf)
return -ENOMEM;
- pathname = d_path(path, tmp, PAGE_SIZE);
+ pathname = d_path(path, buf, PATH_MAX);
len = PTR_ERR(pathname);
if (IS_ERR(pathname))
goto out;
- len = tmp + PAGE_SIZE - 1 - pathname;
+ len = buf + PATH_MAX - 1 - pathname;
if (len > buflen)
len = buflen;
if (copy_to_user(buffer, pathname, len))
len = -EFAULT;
out:
- free_page((unsigned long)tmp);
+ kfree(buf);
return len;
}
--
2.34.1
^ permalink raw reply related [flat|nested] 5+ messages in thread* Re: [PATCH] proc: use kmalloc instead of __get_free_page() to alloc path buffer
2022-01-23 10:08 [PATCH] proc: use kmalloc instead of __get_free_page() to alloc path buffer Hao Lee
@ 2022-01-23 13:55 ` Alexey Dobriyan
2022-01-23 13:58 ` [PATCH v2] proc: alloc PATH_MAX bytes for /proc/${pid}/fd/ symlinks Alexey Dobriyan
2022-01-25 14:03 ` [PATCH] proc: use kmalloc instead of __get_free_page() to alloc path buffer Torin Carey
1 sibling, 1 reply; 5+ messages in thread
From: Alexey Dobriyan @ 2022-01-23 13:55 UTC (permalink / raw)
To: Hao Lee
Cc: akpm, christian.brauner, keescook, jamorris, linux-fsdevel,
linux-kernel
On Sun, Jan 23, 2022 at 10:08:37AM +0000, Hao Lee wrote:
> It's not a standard approach that use __get_free_page() to alloc path
> buffer directly. We'd better use kmalloc and PATH_MAX.
> --- a/fs/proc/base.c
> +++ b/fs/proc/base.c
> @@ -1764,25 +1764,26 @@ static const char *proc_pid_get_link(struct dentry *dentry,
>
> static int do_proc_readlink(struct path *path, char __user *buffer, int buflen)
> {
> - char *tmp = (char *)__get_free_page(GFP_KERNEL);
> + char *buf = NULL;
I'd rather not rename anything but keep it minimal.
^ permalink raw reply [flat|nested] 5+ messages in thread* [PATCH v2] proc: alloc PATH_MAX bytes for /proc/${pid}/fd/ symlinks
2022-01-23 13:55 ` Alexey Dobriyan
@ 2022-01-23 13:58 ` Alexey Dobriyan
2022-01-23 14:23 ` Hao Lee
0 siblings, 1 reply; 5+ messages in thread
From: Alexey Dobriyan @ 2022-01-23 13:58 UTC (permalink / raw)
To: akpm
Cc: christian.brauner, keescook, jamorris, linux-fsdevel,
linux-kernel, Hao Lee
From: Hao Lee <haolee.swjtu@gmail.com>
It's not a standard approach that use __get_free_page() to alloc path
buffer directly. We'd better use kmalloc and PATH_MAX.
PAGE_SIZE is different on different archs. An unlinked file
with very long canonical pathname will readlink differently
because "(deleted)" eats into a buffer. --adobriyan
Signed-off-by: Hao Lee <haolee.swjtu@gmail.com>
Signed-off-by: Alexey Dobriyan <adobriyan@gmail.com>
---
fs/proc/base.c | 8 ++++----
1 file changed, 4 insertions(+), 4 deletions(-)
--- a/fs/proc/base.c
+++ b/fs/proc/base.c
@@ -1764,25 +1764,25 @@ static const char *proc_pid_get_link(struct dentry *dentry,
static int do_proc_readlink(struct path *path, char __user *buffer, int buflen)
{
- char *tmp = (char *)__get_free_page(GFP_KERNEL);
+ char *tmp = (char *)kmalloc(PATH_MAX, GFP_KERNEL);
char *pathname;
int len;
if (!tmp)
return -ENOMEM;
- pathname = d_path(path, tmp, PAGE_SIZE);
+ pathname = d_path(path, tmp, PATH_MAX);
len = PTR_ERR(pathname);
if (IS_ERR(pathname))
goto out;
- len = tmp + PAGE_SIZE - 1 - pathname;
+ len = tmp + PATH_MAX - 1 - pathname;
if (len > buflen)
len = buflen;
if (copy_to_user(buffer, pathname, len))
len = -EFAULT;
out:
- free_page((unsigned long)tmp);
+ kfree(tmp);
return len;
}
^ permalink raw reply [flat|nested] 5+ messages in thread* Re: [PATCH v2] proc: alloc PATH_MAX bytes for /proc/${pid}/fd/ symlinks
2022-01-23 13:58 ` [PATCH v2] proc: alloc PATH_MAX bytes for /proc/${pid}/fd/ symlinks Alexey Dobriyan
@ 2022-01-23 14:23 ` Hao Lee
0 siblings, 0 replies; 5+ messages in thread
From: Hao Lee @ 2022-01-23 14:23 UTC (permalink / raw)
To: Alexey Dobriyan
Cc: Andrew Morton, christian.brauner, Kees Cook, jamorris,
linux-fsdevel, LKML
On Sun, Jan 23, 2022 at 9:58 PM Alexey Dobriyan <adobriyan@gmail.com> wrote:
>
> From: Hao Lee <haolee.swjtu@gmail.com>
>
> It's not a standard approach that use __get_free_page() to alloc path
> buffer directly. We'd better use kmalloc and PATH_MAX.
>
> PAGE_SIZE is different on different archs.
This is indeed a worthy addition. Thanks.
Regards,
Hao Lee
> An unlinked file
> with very long canonical pathname will readlink differently
> because "(deleted)" eats into a buffer. --adobriyan
>
> Signed-off-by: Hao Lee <haolee.swjtu@gmail.com>
> Signed-off-by: Alexey Dobriyan <adobriyan@gmail.com>
> ---
>
> fs/proc/base.c | 8 ++++----
> 1 file changed, 4 insertions(+), 4 deletions(-)
>
> --- a/fs/proc/base.c
> +++ b/fs/proc/base.c
> @@ -1764,25 +1764,25 @@ static const char *proc_pid_get_link(struct dentry *dentry,
>
> static int do_proc_readlink(struct path *path, char __user *buffer, int buflen)
> {
> - char *tmp = (char *)__get_free_page(GFP_KERNEL);
> + char *tmp = (char *)kmalloc(PATH_MAX, GFP_KERNEL);
> char *pathname;
> int len;
>
> if (!tmp)
> return -ENOMEM;
>
> - pathname = d_path(path, tmp, PAGE_SIZE);
> + pathname = d_path(path, tmp, PATH_MAX);
> len = PTR_ERR(pathname);
> if (IS_ERR(pathname))
> goto out;
> - len = tmp + PAGE_SIZE - 1 - pathname;
> + len = tmp + PATH_MAX - 1 - pathname;
>
> if (len > buflen)
> len = buflen;
> if (copy_to_user(buffer, pathname, len))
> len = -EFAULT;
> out:
> - free_page((unsigned long)tmp);
> + kfree(tmp);
> return len;
> }
>
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] proc: use kmalloc instead of __get_free_page() to alloc path buffer
2022-01-23 10:08 [PATCH] proc: use kmalloc instead of __get_free_page() to alloc path buffer Hao Lee
2022-01-23 13:55 ` Alexey Dobriyan
@ 2022-01-25 14:03 ` Torin Carey
1 sibling, 0 replies; 5+ messages in thread
From: Torin Carey @ 2022-01-25 14:03 UTC (permalink / raw)
To: Hao Lee
Cc: akpm, christian.brauner, keescook, adobriyan, jamorris,
linux-fsdevel, linux-kernel
On Sun, Jan 23, 2022 at 10:08:37AM +0000, Hao Lee wrote:
> It's not a standard approach that use __get_free_page() to alloc path
> buffer directly. We'd better use kmalloc and PATH_MAX.
If we're allocating PATH_MAX sized buffers for names, wouldn't it be
suitable to use the name slab names_cachep declared in
include/linux/fs.h using __getname() and __putname()?
Best,
Torin
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2022-01-25 14:14 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2022-01-23 10:08 [PATCH] proc: use kmalloc instead of __get_free_page() to alloc path buffer Hao Lee
2022-01-23 13:55 ` Alexey Dobriyan
2022-01-23 13:58 ` [PATCH v2] proc: alloc PATH_MAX bytes for /proc/${pid}/fd/ symlinks Alexey Dobriyan
2022-01-23 14:23 ` Hao Lee
2022-01-25 14:03 ` [PATCH] proc: use kmalloc instead of __get_free_page() to alloc path buffer Torin Carey
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.