[PATCH v3] libio: null terminate the buffer upon initial allocation in getdelim

[Eric Blake]

On Fri, Nov 28, 2025 at 05:12:23PM -0800, Collin Funk wrote:
> Commit 33eff78c8b28adc4963987880e10d96761f2a167 caused issues in nbdkit
> which had code similar to this to get the last line of the file:
> 
>     while (getline (&line, &len, fp) != -1)
>       ;
>     /* Process LINE.  */
> 
> After that commit, line[0] would be equal to '\0' instead of containing
> the last line of the file like before that commit. A recent POSIX issue
> clarified that the behavior before and after that commit are allowed,
> since the contents of LINE are unspecified after -1 is returned
> [1]. However, some programs rely on the previous behavior.
> 
> This patch null terminates the buffer upon getdelim/getline's initial
> allocation. This is compatible with previous glibc versions, while also
> protecting the caller from reading uninitialized memory if the file is
> empty, as long as getline/getdelim does the initial allocation.
> 
> [1] https://www.austingroupbugs.net/bug_view_page.php?bug_id=1953
> 
> Suggested-by: Eric Blake <eblake@redhat.com>
> ---
>  libio/iogetdelim.c   |  7 +++++--
>  libio/tst-getdelim.c | 47 +++++++++++++++++++++++++++++++++++++-------
>  manual/stdio.texi    |  7 ++++++-
>  3 files changed, 51 insertions(+), 10 deletions(-)

LGTM

> +  /* Test that we can read until -1 is returned and then access the last line
> +     of the file.  This behavior was broken by commit
> +     33eff78c8b28adc4963987880e10d96761f2a167 and later fixed.  */
> +  lineptr = NULL;
> +  linelen = 0;
> +  TEST_VERIFY_EXIT (fwrite ("a\nb\nc\n", 1, sizeof "a\nb\nc\n" - 1, fp)
> +                    == sizeof "a\nb\nc\n" - 1);

Repetitive and harder to refactor, vs. defining the string in a macro
or static array. But since it is on a single line, and since this is a
test, I can live with it; no need to spin v4 on my account.

> +  TEST_VERIFY_EXIT (0 <= fseeko (fp, 0, SEEK_SET));

Would '0 ==' be better than '0 <='?  But even if you keep the weaker
test here, the rest of the test will fail if fseeko() positioned the
file anywhere other than expected, so again, not a reason to require
v4 on my end.

> +  char expect[] = { 'a' - 1, '\n', '\0' };
> +  for (int i = 0; i < 3; ++i)
> +    {
> +      ++expect[0];
> +      TEST_VERIFY (getdelim (&lineptr, &linelen, '\n', fp) == 2);
> +      TEST_VERIFY (linelen > 0);

Stronger would be TEST_VERIFY (linelen > 2) (that is, on success,
linelen MUST be larger than the result that getdelim just returned, to
account for the trailing null byte that must be appended after the
length of the string returned).

> +      TEST_VERIFY (strcmp (lineptr, expect) == 0);

Stronger might be strncmp (lineptr, expect, linelen) or even memcmp
(lineptr, expect, linelen+1) (assuming you also check that linelen >
strlen (expect)).

> +    }

The reason I mention a stronger test is that a bug in the
implementation where line is not (re)alloced to something large enough
could be masked if glibc wrote beyond the end of the buffer, but the
strcmp still succeeds.

But in the interest of getting this patch in, I'm okay with accepting
the test as written.


-- 
Eric Blake, Principal Software Engineer
Red Hat, Inc.
Virtualization:  qemu.org | libguestfs.org