Add two more checks for buflen and numwritten. The buflen should be at
least one, otherwise the 'buflen - 1' could underflow and cause trouble.
The numwritten should be equal to 'buflen - 1'. The test will exit if
any of these conditions aren't met.

Additionally, add more print information when a write failure occurs or
a truncated write happens, providing clearer diagnostics.

Signed-off-by: Chunyu Hu <chuhu@redhat.com>
---
Changes in v6:
  - save/restore errno before and after the close(), so the log could show
    the correct error info on failure. Suggested by AI.
Chagnes in v5:
  - new patch for making improve on write_file. Add more safety checks and
    diagnostics info in log
---
 tools/testing/selftests/mm/vm_util.c | 12 ++++++++++--
 1 file changed, 10 insertions(+), 2 deletions(-)

diff --git a/tools/testing/selftests/mm/vm_util.c b/tools/testing/selftests/mm/vm_util.c
index ad96d19d1b85..572ccb99de8e 100644
--- a/tools/testing/selftests/mm/vm_util.c
+++ b/tools/testing/selftests/mm/vm_util.c
@@ -767,15 +767,23 @@ int unpoison_memory(unsigned long pfn)
 
 void write_file(const char *path, const char *buf, size_t buflen)
 {
-	int fd;
+	int fd, saved_errno;
 	ssize_t numwritten;
+	if (buflen < 1)
+		ksft_exit_fail_msg("Incorrect buffer len: %zu\n", buflen);
 
 	fd = open(path, O_WRONLY);
 	if (fd == -1)
 		ksft_exit_fail_msg("%s open failed: %s\n", path, strerror(errno));
 
 	numwritten = write(fd, buf, buflen - 1);
+	saved_errno = errno;
 	close(fd);
+	errno = saved_errno;
 	if (numwritten < 1)
-		ksft_exit_fail_msg("Write failed\n");
+		ksft_exit_fail_msg("%s write(%s) failed: %s\n", path, buf,
+				strerror(errno));
+	if (numwritten != buflen - 1)
+		ksft_exit_fail_msg("%s write(%s) is truncated, expected %zu bytes, got %zd bytes\n",
+				path, buf, buflen - 1, numwritten);
 }
-- 
2.53.0

On Tue, Mar 24, 2026 at 09:33:14AM +0800, Chunyu Hu wrote:
> Add two more checks for buflen and numwritten. The buflen should be at
> least one, otherwise the 'buflen - 1' could underflow and cause trouble.
> The numwritten should be equal to 'buflen - 1'. The test will exit if
> any of these conditions aren't met.
> 
> Additionally, add more print information when a write failure occurs or
> a truncated write happens, providing clearer diagnostics.
> 
> Signed-off-by: Chunyu Hu <chuhu@redhat.com>
> ---
> Changes in v6:
>   - save/restore errno before and after the close(), so the log could show
>     the correct error info on failure. Suggested by AI.
> Chagnes in v5:
>   - new patch for making improve on write_file. Add more safety checks and
>     diagnostics info in log
> ---
>  tools/testing/selftests/mm/vm_util.c | 12 ++++++++++--
>  1 file changed, 10 insertions(+), 2 deletions(-)
> 
> diff --git a/tools/testing/selftests/mm/vm_util.c b/tools/testing/selftests/mm/vm_util.c
> index ad96d19d1b85..572ccb99de8e 100644
> --- a/tools/testing/selftests/mm/vm_util.c
> +++ b/tools/testing/selftests/mm/vm_util.c
> @@ -767,15 +767,23 @@ int unpoison_memory(unsigned long pfn)
>  
>  void write_file(const char *path, const char *buf, size_t buflen)
>  {
> -	int fd;
> +	int fd, saved_errno;
>  	ssize_t numwritten;
> +	if (buflen < 1)
> +		ksft_exit_fail_msg("Incorrect buffer len: %zu\n", buflen);
>  
>  	fd = open(path, O_WRONLY);
>  	if (fd == -1)
>  		ksft_exit_fail_msg("%s open failed: %s\n", path, strerror(errno));

We have ksft_exit_fail_perror(), fits great here

>  
>  	numwritten = write(fd, buf, buflen - 1);
> +	saved_errno = errno;
>  	close(fd);
> +	errno = saved_errno;
>  	if (numwritten < 1)
> -		ksft_exit_fail_msg("Write failed\n");
> +		ksft_exit_fail_msg("%s write(%s) failed: %s\n", path, buf,
> +				strerror(errno));

and here.

> +	if (numwritten != buflen - 1)
> +		ksft_exit_fail_msg("%s write(%s) is truncated, expected %zu bytes, got %zd bytes\n",
> +				path, buf, buflen - 1, numwritten);
>  }
> -- 
> 2.53.0
> 

-- 
Sincerely yours,
Mike.

On Fri, Mar 27, 2026 at 03:32:01PM +0300, Mike Rapoport wrote:
> On Tue, Mar 24, 2026 at 09:33:14AM +0800, Chunyu Hu wrote:
> > Add two more checks for buflen and numwritten. The buflen should be at
> > least one, otherwise the 'buflen - 1' could underflow and cause trouble.
> > The numwritten should be equal to 'buflen - 1'. The test will exit if
> > any of these conditions aren't met.
> > 
> > Additionally, add more print information when a write failure occurs or
> > a truncated write happens, providing clearer diagnostics.
> > 
> > Signed-off-by: Chunyu Hu <chuhu@redhat.com>
> > ---
> > Changes in v6:
> >   - save/restore errno before and after the close(), so the log could show
> >     the correct error info on failure. Suggested by AI.
> > Chagnes in v5:
> >   - new patch for making improve on write_file. Add more safety checks and
> >     diagnostics info in log
> > ---
> >  tools/testing/selftests/mm/vm_util.c | 12 ++++++++++--
> >  1 file changed, 10 insertions(+), 2 deletions(-)
> > 
> > diff --git a/tools/testing/selftests/mm/vm_util.c b/tools/testing/selftests/mm/vm_util.c
> > index ad96d19d1b85..572ccb99de8e 100644
> > --- a/tools/testing/selftests/mm/vm_util.c
> > +++ b/tools/testing/selftests/mm/vm_util.c
> > @@ -767,15 +767,23 @@ int unpoison_memory(unsigned long pfn)
> >  
> >  void write_file(const char *path, const char *buf, size_t buflen)
> >  {
> > -	int fd;
> > +	int fd, saved_errno;
> >  	ssize_t numwritten;
> > +	if (buflen < 1)
> > +		ksft_exit_fail_msg("Incorrect buffer len: %zu\n", buflen);
> >  
> >  	fd = open(path, O_WRONLY);
> >  	if (fd == -1)
> >  		ksft_exit_fail_msg("%s open failed: %s\n", path, strerror(errno));
> 
> We have ksft_exit_fail_perror(), fits great here

the ksft_exit_fail_perror() don't accept the fmt string, it only accept
a single string as parameter.

```
static inline __noreturn void ksft_exit_fail_perror(const char *msg)

```

But we can change it also support va_list arg like 
ksft_exit_fail_msg(msg, ...) does:

static inline __noreturn void ksft_exit_fail_perror(msg, ...)

I can have a try in next version.

> 
> >  
> >  	numwritten = write(fd, buf, buflen - 1);
> > +	saved_errno = errno;
> >  	close(fd);
> > +	errno = saved_errno;
> >  	if (numwritten < 1)
> > -		ksft_exit_fail_msg("Write failed\n");
> > +		ksft_exit_fail_msg("%s write(%s) failed: %s\n", path, buf,
> > +				strerror(errno));
> 
> and here.

Same answer with above coment.

> 
> > +	if (numwritten != buflen - 1)
> > +		ksft_exit_fail_msg("%s write(%s) is truncated, expected %zu bytes, got %zd bytes\n",
> > +				path, buf, buflen - 1, numwritten);
> >  }
> > -- 
> > 2.53.0
> > 
> 
> -- 
> Sincerely yours,
> Mike.
>

On Sun, Mar 29, 2026 at 05:27:13PM +0800, Chunyu Hu wrote:
> On Fri, Mar 27, 2026 at 03:32:01PM +0300, Mike Rapoport wrote:
> > On Tue, Mar 24, 2026 at 09:33:14AM +0800, Chunyu Hu wrote:
> > > Add two more checks for buflen and numwritten. The buflen should be at
> > > least one, otherwise the 'buflen - 1' could underflow and cause trouble.
> > > The numwritten should be equal to 'buflen - 1'. The test will exit if
> > > any of these conditions aren't met.
> > > 
> > > Additionally, add more print information when a write failure occurs or
> > > a truncated write happens, providing clearer diagnostics.
> > > 
> > > Signed-off-by: Chunyu Hu <chuhu@redhat.com>
> > > ---
> > > Changes in v6:
> > >   - save/restore errno before and after the close(), so the log could show
> > >     the correct error info on failure. Suggested by AI.
> > > Chagnes in v5:
> > >   - new patch for making improve on write_file. Add more safety checks and
> > >     diagnostics info in log
> > > ---
> > >  tools/testing/selftests/mm/vm_util.c | 12 ++++++++++--
> > >  1 file changed, 10 insertions(+), 2 deletions(-)
> > > 
> > > diff --git a/tools/testing/selftests/mm/vm_util.c b/tools/testing/selftests/mm/vm_util.c
> > > index ad96d19d1b85..572ccb99de8e 100644
> > > --- a/tools/testing/selftests/mm/vm_util.c
> > > +++ b/tools/testing/selftests/mm/vm_util.c
> > > @@ -767,15 +767,23 @@ int unpoison_memory(unsigned long pfn)
> > >  
> > >  void write_file(const char *path, const char *buf, size_t buflen)
> > >  {
> > > -	int fd;
> > > +	int fd, saved_errno;
> > >  	ssize_t numwritten;
> > > +	if (buflen < 1)
> > > +		ksft_exit_fail_msg("Incorrect buffer len: %zu\n", buflen);
> > >  
> > >  	fd = open(path, O_WRONLY);
> > >  	if (fd == -1)
> > >  		ksft_exit_fail_msg("%s open failed: %s\n", path, strerror(errno));
> > 
> > We have ksft_exit_fail_perror(), fits great here
> 
> the ksft_exit_fail_perror() don't accept the fmt string, it only accept
> a single string as parameter.
> 
> ```
> static inline __noreturn void ksft_exit_fail_perror(const char *msg)
> 
> ```
> 
> But we can change it also support va_list arg like 
> ksft_exit_fail_msg(msg, ...) does:
> 
> static inline __noreturn void ksft_exit_fail_perror(msg, ...)
> 
> I can have a try in next version.

Tried in v7.

In v7, I tried to change ksft_exit_fail_perror to support va_list args,
and I used vasrpintf() to print the message to the buf and then pass it
to ksft_exit_fail_msg(). But it requires _GNU_SOURCE can cause a build
issue in mm-unstable tree from the kernel test robot report. 

The vasprintf, the static buf[] (will trucate long message), or make
a copy of body of the ksft_exit_fail_msg and append a perror string,
none of these ways is clean, using ksft_exit_fail_msg may be still
a better way.

What do you think, Mike?

> 
> > 
> > >  
> > >  	numwritten = write(fd, buf, buflen - 1);
> > > +	saved_errno = errno;
> > >  	close(fd);
> > > +	errno = saved_errno;
> > >  	if (numwritten < 1)
> > > -		ksft_exit_fail_msg("Write failed\n");
> > > +		ksft_exit_fail_msg("%s write(%s) failed: %s\n", path, buf,
> > > +				strerror(errno));
> > 
> > and here.
> 
> Same answer with above coment.
> 
> > 
> > > +	if (numwritten != buflen - 1)
> > > +		ksft_exit_fail_msg("%s write(%s) is truncated, expected %zu bytes, got %zd bytes\n",
> > > +				path, buf, buflen - 1, numwritten);
> > >  }
> > > -- 
> > > 2.53.0
> > > 
> > 
> > -- 
> > Sincerely yours,
> > Mike.
> > 
>

On Tue, Mar 24, 2026 at 09:33:14AM +0800, Chunyu Hu wrote:
> Add two more checks for buflen and numwritten. The buflen should be at
> least one, otherwise the 'buflen - 1' could underflow and cause trouble.
> The numwritten should be equal to 'buflen - 1'. The test will exit if
> any of these conditions aren't met.
>
> Additionally, add more print information when a write failure occurs or
> a truncated write happens, providing clearer diagnostics.
>
> Signed-off-by: Chunyu Hu <chuhu@redhat.com>
> ---
> Changes in v6:
>   - save/restore errno before and after the close(), so the log could show
>     the correct error info on failure. Suggested by AI.
> Chagnes in v5:
>   - new patch for making improve on write_file. Add more safety checks and
>     diagnostics info in log
> ---
>  tools/testing/selftests/mm/vm_util.c | 12 ++++++++++--
>  1 file changed, 10 insertions(+), 2 deletions(-)
>
> diff --git a/tools/testing/selftests/mm/vm_util.c b/tools/testing/selftests/mm/vm_util.c
> index ad96d19d1b85..572ccb99de8e 100644
> --- a/tools/testing/selftests/mm/vm_util.c
> +++ b/tools/testing/selftests/mm/vm_util.c
> @@ -767,15 +767,23 @@ int unpoison_memory(unsigned long pfn)
>
>  void write_file(const char *path, const char *buf, size_t buflen)
>  {
> -	int fd;
> +	int fd, saved_errno;
>  	ssize_t numwritten;
> +	if (buflen < 1)

As per Sashiko (presumably didn't look :P), let's make this < 2 since we
write buflen - 1 bytes below which we don't ever want to be 0.

> +		ksft_exit_fail_msg("Incorrect buffer len: %zu\n", buflen);
>
>  	fd = open(path, O_WRONLY);
>  	if (fd == -1)
>  		ksft_exit_fail_msg("%s open failed: %s\n", path, strerror(errno));
>
>  	numwritten = write(fd, buf, buflen - 1);
> +	saved_errno = errno;
>  	close(fd);
> +	errno = saved_errno;
>  	if (numwritten < 1)
> -		ksft_exit_fail_msg("Write failed\n");
> +		ksft_exit_fail_msg("%s write(%s) failed: %s\n", path, buf,
> +				strerror(errno));
> +	if (numwritten != buflen - 1)
> +		ksft_exit_fail_msg("%s write(%s) is truncated, expected %zu bytes, got %zd bytes\n",
> +				path, buf, buflen - 1, numwritten);
>  }
> --
> 2.53.0
>

On Fri, Mar 27, 2026 at 07:46:03AM +0000, Lorenzo Stoakes (Oracle) wrote:
> On Tue, Mar 24, 2026 at 09:33:14AM +0800, Chunyu Hu wrote:
> > Add two more checks for buflen and numwritten. The buflen should be at
> > least one, otherwise the 'buflen - 1' could underflow and cause trouble.
> > The numwritten should be equal to 'buflen - 1'. The test will exit if
> > any of these conditions aren't met.
> >
> > Additionally, add more print information when a write failure occurs or
> > a truncated write happens, providing clearer diagnostics.
> >
> > Signed-off-by: Chunyu Hu <chuhu@redhat.com>
> > ---
> > Changes in v6:
> >   - save/restore errno before and after the close(), so the log could show
> >     the correct error info on failure. Suggested by AI.
> > Chagnes in v5:
> >   - new patch for making improve on write_file. Add more safety checks and
> >     diagnostics info in log
> > ---
> >  tools/testing/selftests/mm/vm_util.c | 12 ++++++++++--
> >  1 file changed, 10 insertions(+), 2 deletions(-)
> >
> > diff --git a/tools/testing/selftests/mm/vm_util.c b/tools/testing/selftests/mm/vm_util.c
> > index ad96d19d1b85..572ccb99de8e 100644
> > --- a/tools/testing/selftests/mm/vm_util.c
> > +++ b/tools/testing/selftests/mm/vm_util.c
> > @@ -767,15 +767,23 @@ int unpoison_memory(unsigned long pfn)
> >
> >  void write_file(const char *path, const char *buf, size_t buflen)
> >  {
> > -	int fd;
> > +	int fd, saved_errno;
> >  	ssize_t numwritten;
> > +	if (buflen < 1)
> 
> As per Sashiko (presumably didn't look :P), let's make this < 2 since we
> write buflen - 1 bytes below which we don't ever want to be 0.

That makes sense. Let me change to 2 in next version. Thanks!

> 
> > +		ksft_exit_fail_msg("Incorrect buffer len: %zu\n", buflen);
> >
> >  	fd = open(path, O_WRONLY);
> >  	if (fd == -1)
> >  		ksft_exit_fail_msg("%s open failed: %s\n", path, strerror(errno));
> >
> >  	numwritten = write(fd, buf, buflen - 1);
> > +	saved_errno = errno;
> >  	close(fd);
> > +	errno = saved_errno;
> >  	if (numwritten < 1)
> > -		ksft_exit_fail_msg("Write failed\n");
> > +		ksft_exit_fail_msg("%s write(%s) failed: %s\n", path, buf,
> > +				strerror(errno));
> > +	if (numwritten != buflen - 1)
> > +		ksft_exit_fail_msg("%s write(%s) is truncated, expected %zu bytes, got %zd bytes\n",
> > +				path, buf, buflen - 1, numwritten);
> >  }
> > --
> > 2.53.0
> >
>

On Tue, 24 Mar 2026 09:33:14 +0800 Chunyu Hu <chuhu@redhat.com> wrote:

> Add two more checks for buflen and numwritten. The buflen should be at
> least one, otherwise the 'buflen - 1' could underflow and cause trouble.
> The numwritten should be equal to 'buflen - 1'. The test will exit if
> any of these conditions aren't met.
> 
> Additionally, add more print information when a write failure occurs or
> a truncated write happens, providing clearer diagnostics.

This one has no review and AI review did identify a kinda-bug.  Maybe
nobody calls write_file() with buflen==1, but still worth addressing I
think.

https://sashiko.dev/#/patchset/20260324013316.2590422-1-chuhu%40redhat.com

Chunyu, could you please take a look?

Thanks.

On Thu, Mar 26, 2026 at 11:03:49PM -0700, Andrew Morton wrote:
> On Tue, 24 Mar 2026 09:33:14 +0800 Chunyu Hu <chuhu@redhat.com> wrote:
> 
> > Add two more checks for buflen and numwritten. The buflen should be at
> > least one, otherwise the 'buflen - 1' could underflow and cause trouble.
> > The numwritten should be equal to 'buflen - 1'. The test will exit if
> > any of these conditions aren't met.
> > 
> > Additionally, add more print information when a write failure occurs or
> > a truncated write happens, providing clearer diagnostics.
> 
> This one has no review and AI review did identify a kinda-bug.  Maybe
> nobody calls write_file() with buflen==1, but still worth addressing I
> think.
> 
> https://sashiko.dev/#/patchset/20260324013316.2590422-1-chuhu%40redhat.com
> 
> Chunyu, could you please take a look?

Andrew,

By limiting the buflen at least 2, it will avoid such an issue, this is
also suggested by Lorenzo. I'll fix this in next version. 

AI question about that we need to handle error path and trucate path
(numwritten < 0) and (numwritten < buflen -1) seperately.  That makes
sense. I'll handle that in next version.

"""
> +	if (numwritten != buflen - 1)
> +		ksft_exit_fail_msg("%s write(%s) is truncated, expected %zu bytes, got %zd bytes\n",
> +				path, buf, buflen - 1, numwritten);
>  }
This isn't a bug, but the new error messages here and above use %s to
format buf. Since the API requires an explicit buflen parameter and writes
buflen - 1 bytes, does using %s risk out-of-bounds reads if a caller
passes a buffer that is not null-terminated?
Would it be safer to use a precision specifier like %.*s and pass
(int)(buflen - 1) as the length to safely print the exact contents intended
to be written?
"""

That makes sense, I'll use %.*s instead for the buf print in next
version.

> 
> Thanks.
>