[PATCH] scripts/kallsyms: Fix build failure by setting errno before calling getline()

[PATCH] scripts/kallsyms: Fix build failure by setting errno before calling getline()

[James Clark]

getline() returns -1 at EOF as well as on error. It also doesn't set
errno to 0 on success, so initialize it to 0 before using errno to check
for an error condition. See the paragraph here [1]:

  For some system calls and library functions (e.g., getpriority(2)),
  -1 is a valid return on success. In such cases, a successful return
  can be distinguished from an error return by setting errno to zero
  before the call, and then, if the call returns a status that indicates
  that an error may have occurred, checking to see if errno has a
  nonzero value.

This fixes the following build failure if scripts/kallsyms launches with
a non-zero errno value:

 $ make
 ...
  LD      .tmp_vmlinux.kallsyms1
  NM      .tmp_vmlinux.kallsyms1.syms
  KSYMS   .tmp_vmlinux.kallsyms1.S
 read_symbol: Invalid argument

[1]: https://linux.die.net/man/3/errno

Fixes: 1c975da56a6f ("scripts/kallsyms: remove KSYM_NAME_LEN_BUFFER")
Signed-off-by: James Clark <james.clark@arm.com>
---
 scripts/kallsyms.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/scripts/kallsyms.c b/scripts/kallsyms.c
index 16c87938b316..653b92f6d4c8 100644
--- a/scripts/kallsyms.c
+++ b/scripts/kallsyms.c
@@ -129,6 +129,7 @@ static struct sym_entry *read_symbol(FILE *in, char **buf, size_t *buf_len)
 	ssize_t readlen;
 	struct sym_entry *sym;
 
+	errno = 0;
 	readlen = getline(buf, buf_len, in);
 	if (readlen < 0) {
 		if (errno) {
-- 
2.34.1

Re: [PATCH] scripts/kallsyms: Fix build failure by setting errno before calling getline()

[Miguel Ojeda]

On Mon, Jul 24, 2023 at 3:18 PM James Clark <james.clark@arm.com> wrote:
>
> This fixes the following build failure if scripts/kallsyms launches with
> a non-zero errno value:

The code change sounds good to me, but could you please describe the
situation where you found the build failure?

Cheers,
Miguel

Re: [PATCH] scripts/kallsyms: Fix build failure by setting errno before calling getline()

[James Clark]

On 25/07/2023 01:41, Miguel Ojeda wrote:
> On Mon, Jul 24, 2023 at 3:18 PM James Clark <james.clark@arm.com> wrote:
>>
>> This fixes the following build failure if scripts/kallsyms launches with
>> a non-zero errno value:
> 
> The code change sounds good to me, but could you please describe the
> situation where you found the build failure?
> 
> Cheers,
> Miguel

I assumed it was something to do with one of the wrappers I'm using but
didn't get to the bottom of it because I'm using quite a few.

But I just checked now and it's just bear [1] that causes the issue.
Maybe it sets errno before forking and that persists in the child
processes? Not 100% sure how it works. I did debug scripts/kallsyms and
errno was already set on the first line of main(), so it's not some
other library call in kallsyms that is setting it but not being checked.

The minimal reproducer for me looks like this:

  bear -- make ARCH=arm64 LLVM=1 O=../build/

[1]: https://manpages.ubuntu.com/manpages/focal/en/man1/bear.1.html

Re: [PATCH] scripts/kallsyms: Fix build failure by setting errno before calling getline()

[Miguel Ojeda]

On Tue, Jul 25, 2023 at 10:55 AM James Clark <james.clark@arm.com> wrote:
>
> But I just checked now and it's just bear [1] that causes the issue.
> Maybe it sets errno before forking and that persists in the child
> processes? Not 100% sure how it works. I did debug scripts/kallsyms and
> errno was already set on the first line of main(), so it's not some
> other library call in kallsyms that is setting it but not being checked.

I think this is https://github.com/rizsotto/Bear/issues/469: bear
preloads a library which was setting it to non-zero.

Now, even if bear was breaking a guarantee C programs have (`errno`
for the initial thread at program startup is meant to be 0 according
to the C standard), it is still a good idea to set `errno` to zero
here in case something else happens to change `errno` within
`kallsyms` in the future.

With the repro and the `bear` link issue added:

    Reviewed-by: Miguel Ojeda <ojeda@kernel.org>

Cheers,
Miguel

Re: [PATCH] scripts/kallsyms: Fix build failure by setting errno before calling getline()

[James Clark]

On 25/07/2023 11:33, Miguel Ojeda wrote:
> On Tue, Jul 25, 2023 at 10:55 AM James Clark <james.clark@arm.com> wrote:
>>
>> But I just checked now and it's just bear [1] that causes the issue.
>> Maybe it sets errno before forking and that persists in the child
>> processes? Not 100% sure how it works. I did debug scripts/kallsyms and
>> errno was already set on the first line of main(), so it's not some
>> other library call in kallsyms that is setting it but not being checked.
> 
> I think this is https://github.com/rizsotto/Bear/issues/469: bear
> preloads a library which was setting it to non-zero.
> 
> Now, even if bear was breaking a guarantee C programs have (`errno`
> for the initial thread at program startup is meant to be 0 according
> to the C standard), it is still a good idea to set `errno` to zero
> here in case something else happens to change `errno` within
> `kallsyms` in the future.
> 
> With the repro and the `bear` link issue added:
> 
>     Reviewed-by: Miguel Ojeda <ojeda@kernel.org>
> 
> Cheers,
> Miguel

Oh yeah nice find, that's the issue. I added it and sent a v2.

Thanks
James