Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Include header for basename() #14

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

danobi
Copy link

@danobi danobi commented Dec 6, 2024

bpftrace has an appimage build which we configure through nix. Recently we upgraded from nix 24.05 to nix 24.11. After the upgrade, we started seeing a segfault on appimage startup:

Program received signal SIGSEGV, Segmentation fault.
0x0000000000477598 in strlen ()
(gdb) where
#0  0x0000000000477598 in strlen ()
#1  0x0000000000403c46 in build_mount_point ()
#2  0x00000000004016f8 in main ()

Disassembling build_mount_point() reveals the following:

0x0000000000403c36 <+22>:    call   0x471833 <basename>
0x0000000000403c3b <+27>:    movslq %eax,%r13
0x0000000000403c3e <+30>:    mov    %r13,%rdi
0x0000000000403c41 <+33>:    call   0x47758b <strlen>

That is, the call to basename() is on a valid string (I checked). But following the call, the return value is read from %eax as a 32-bit value and then extend moved into 64-bit %rdi - argument 1 for the subsequent call to strlen() - as specified by System V ABI for x86-64.

Obviously truncating a 64-bit pointer into 32-bits and then sign extending it into a 64-bit value is not correct.

I turned to the build logs to check for clues. That's where I found this warning:

/nix/store/llifij4cb8171wy2y2a74wxv53v636zr-source/src/main.c: In
function 'build_mount_point':
/nix/store/llifij4cb8171wy2y2a74wxv53v636zr-source/src/main.c:781:21:
warning: implicit declaration of function 'basename'
[-Wimplicit-function-declaration]
  781 |     path_basename = basename(argv0);
      |                     ^~~~~~~~

/nix/store/llifij4cb8171wy2y2a74wxv53v636zr-source/src/main.c:781:19:
warning: assignment to 'char *' from 'int' makes pointer from
integer without a cast [-Wint-conversion]
  781 |     path_basename = basename(argv0);
      |                   ^

It turns out the prototype for basename() was missing so the compiler defaulted to treating the return as an int. Which explains the pointer truncation. The annoying thing is nix hides build warnings by default. Otherwise I probably would've noticed earlier.

The fix is to include the proper header. The segfault goes away after doing so.

bpftrace has an appimage build which we configure through nix. Recently
we upgraded from nix 24.05 to nix 24.11. After the upgrade, we started
seeing a segfault on appimage startup:

    Program received signal SIGSEGV, Segmentation fault.
    0x0000000000477598 in strlen ()
    (gdb) where
    #0  0x0000000000477598 in strlen ()
    AppImageCrafters#1  0x0000000000403c46 in build_mount_point ()
    AppImageCrafters#2  0x00000000004016f8 in main ()

Disassembling build_mount_point() reveals the following:

    0x0000000000403c36 <+22>:    call   0x471833 <basename>
    0x0000000000403c3b <+27>:    movslq %eax,%r13
    0x0000000000403c3e <+30>:    mov    %r13,%rdi
    0x0000000000403c41 <+33>:    call   0x47758b <strlen>

That is, the call to basename() is on a valid string (I checked). But
following the call, the return value is read from %eax as a 32-bit value
and then extend moved into 64-bit %rdi - argument 1 for the subsequent
call to strlen() - as specified by System V ABI for x86-64.

Obviously truncating a 64-bit pointer into 32-bits and then sign
extending it into a 64-bit value is not correct.

I turned to the build logs to check for clues. That's where I found this
warning:

    /nix/store/llifij4cb8171wy2y2a74wxv53v636zr-source/src/main.c: In
    function 'build_mount_point':
    /nix/store/llifij4cb8171wy2y2a74wxv53v636zr-source/src/main.c:781:21:
    warning: implicit declaration of function 'basename'
    [-Wimplicit-function-declaration]
      781 |     path_basename = basename(argv0);
          |                     ^~~~~~~~

    /nix/store/llifij4cb8171wy2y2a74wxv53v636zr-source/src/main.c:781:19:
    warning: assignment to 'char *' from 'int' makes pointer from
    integer without a cast [-Wint-conversion]
      781 |     path_basename = basename(argv0);
          |                   ^

It turns out the prototype for basename() was missing so the compiler
defaulted to treating the return as an int. Which explains the
pointer truncation. The annoying thing is nix hides build warnings by
default. Otherwise I probably would've noticed earlier.

The fix is to include the proper header. The segfault goes away after
doing so.
danobi added a commit to danobi/nix-appimage that referenced this pull request Dec 6, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant