Re: [PATCH] mm: huge_memory: Handle strsep not finding delimiter

[Andrew Morton <akpm@linux-foundation.org>]

On Sun, 15 Dec 2024 20:27:51 -0800 Leo Stone <leocstone@gmail.com> wrote:

> split_huge_pages_write does not handle the case where strsep finds no
> delimiter in the given string and sets the input buffer to NULL,
> which allows this reproducer to trigger a protection fault.
> 
> ...
>
> --- a/mm/huge_memory.c
> +++ b/mm/huge_memory.c
> @@ -4168,7 +4168,7 @@ static ssize_t split_huge_pages_write(struct file *file, const char __user *buf,
>  		size_t input_len = strlen(input_buf);
>  
>  		tok = strsep(&buf, ",");
> -		if (tok) {
> +		if (tok && buf) {
>  			strscpy(file_path, tok);
>  		} else {
>  			ret = -EINVAL;

lgtm, thanks.

The duplicated `buf' made review of this unnecessarily annoying, so...


From: Andrew Morton <akpm@linux-foundation.org>
Subject: mm/huge_memory.c: rename shadowed local
Date: Sun Dec 15 09:44:47 PM PST 2024

split_huge_pages_write() has a lccal `buf' which shadows incoming arg
`buf'.  Reviewer confusion resulted.

Cc: Leo Stone <leocstone@gmail.com>
Signed-off-by: Andrew Morton <akpm@linux-foundation.org>
---

 mm/huge_memory.c |    9 +++++----
 1 file changed, 5 insertions(+), 4 deletions(-)

--- a/mm/huge_memory.c~mm-huge_memoryc-rename-shadowed-local
+++ a/mm/huge_memory.c
@@ -4169,20 +4169,21 @@ static ssize_t split_huge_pages_write(st
 
 	if (input_buf[0] == '/') {
 		char *tok;
-		char *buf = input_buf;
+		char *tok_buf = input_buf;
 		char file_path[MAX_INPUT_BUF_SZ];
 		pgoff_t off_start = 0, off_end = 0;
 		size_t input_len = strlen(input_buf);
 
-		tok = strsep(&buf, ",");
-		if (tok && buf) {
+		tok = strsep(&tok_buf, ",");
+		if (tok && tok_buf) {
 			strscpy(file_path, tok);
 		} else {
 			ret = -EINVAL;
 			goto out;
 		}
 
-		ret = sscanf(buf, "0x%lx,0x%lx,%d", &off_start, &off_end, &new_order);
+		ret = sscanf(tok_buf, "0x%lx,0x%lx,%d", &off_start,
+			    &off_end, &new_order);
 		if (ret != 2 && ret != 3) {
 			ret = -EINVAL;
 			goto out;
_