[PATCH v2] libio: Fix fmemopen_write on appending condition

[Adhemerval Zanella Netto]

On 23/03/26 05:22, Rocket Ma wrote:
> * libio/fmemopen.c: reference pos the variable instead of c->pos
> 
> To reproduce, write a C unit with the following code and then compile it
> and run it, should expect an output with "1 No space left on device".
> Apparently, for appending mode, there is enough room to write some
> bytes, but it didn't. `c->pos` should be corrected to `pos`.
> 
> Signed-off-by: Rocket Ma <marocketbd@gmail.com>
> ---
>  libio/Makefile       |  1 +
>  libio/bug-fmemopen.c | 22 ++++++++++++++++++++++
>  libio/fmemopen.c     |  2 +-
>  3 files changed, 24 insertions(+), 1 deletion(-)
>  create mode 100644 libio/bug-fmemopen.c
> 
> diff --git a/libio/Makefile b/libio/Makefile
> index 08e1e0ec25..c60ebf800d 100644
> --- a/libio/Makefile
> +++ b/libio/Makefile
> @@ -70,6 +70,7 @@ routines_no_fortify += \
>    # routines_no_fortify
>  
>  tests = \
> +  bug-fmemopen \
>    bug-fopena+ \
>    bug-fseek \
>    bug-ftell \
> diff --git a/libio/bug-fmemopen.c b/libio/bug-fmemopen.c
> new file mode 100644
> index 0000000000..923212ee1b
> --- /dev/null
> +++ b/libio/bug-fmemopen.c
> @@ -0,0 +1,22 @@

This test is missing a Copyright and I think it should be placed on stdio-common/
subfolder.

> +#include <stdio.h>
> +
> +#define tst_assert(cond)                                                      \
> +  if (!(cond))                                                                \
> +    {                                                                         \
> +      puts ("Failed assertion: " #cond);                                      \
> +      return 1;                                                               \
> +    }

We prefer to use support/check.h macros.

> +
> +static int
> +do_test (void)
> +{
> +  char buf[5] = "1";
> +  FILE *fp = fmemopen (buf, 4, "a+");
> +  tst_assert (fp != NULL);
> +  tst_assert (fseek (fp, 3, SEEK_SET) == 0);
> +  tst_assert (fwrite ("XXXX", 1, 4, fp) > 0);

This test does not really stress the issue, it works regardless of the patch
is applied or not.  We need also to check if the string does have the XXX append
on it:

static int
do_test (void)
{
  char buf[5] = "1";
  FILE *fp = xfmemopen (buf, 4, "a+");
  TEST_COMPARE (fseek (fp, 3, SEEK_SET), 0);
  TEST_VERIFY (fwrite ("XXXX", 1, 4, fp) > 0);
  int r = fclose (fp);
  printf ("r=%d errno=%s\n", r, strerrorname_np (errno));
  TEST_COMPARE_STRING (buf, "1XXX");

  return 0;
}

> +
> +  return 0;
> +}
> +
> +#include <support/test-driver.c>
> diff --git a/libio/fmemopen.c b/libio/fmemopen.c
> index f2ae1338d3..cdc3a3476e 100644
> --- a/libio/fmemopen.c
> +++ b/libio/fmemopen.c
> @@ -71,7 +71,7 @@ fmemopen_write (void *cookie, const char *b, size_t s)
>  
>    if (pos + s > c->size)
>      {
> -      if ((size_t) (c->pos + addnullc) >= c->size)
> +      if ((size_t) (pos + addnullc) >= c->size)
>  	{
>  	  __set_errno (ENOSPC);
>  	  return 0;

I think this does not fully fix the issue, since the fwrite below
will still return 4 where I would expect 3 bytes are written 
(buf becomes "1XXX"), the null terminator attempt lands at size = 4 
(boundary, not within the array). 

Unfortunately, the current glibc implementation only calls
fmemopen_write at flush, and thus the fclose above will fail without
setting the errno. At least with current code we properly fail with
ENOSPC in such cases.

I think we will need to proper fix the fwrite return code before,
so fflush/fclose does not fail in this case.