| Message ID | 20230724131741.954624-1-james.clark@arm.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | scripts/kallsyms: Fix build failure by setting errno before calling getline() | expand |
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
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
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
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
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) {
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(+)