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.

Sign up for GitHub

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

Jump to bottom

Fix newstat and newfstat syscalls #2544

Merged
merged 1 commit into from
May 4, 2022
Merged

Fix newstat and newfstat syscalls #2544

merged 1 commit into from
May 4, 2022

Conversation

lordidiot
Copy link
Contributor

Two changes in this PR

newstat

  • Currently sys_newstat's implementation is based on sys_stat64
  • This is inaccurate because sys_stat64 is expected to return a struct stat64 back to userland ref1 ref2
  • Instead, sys_newstat is supposed to return a struct stat ref1 ref2
  • This causes issues because the two structs have different definitions, and in practice causes errors in the loader. For example "LD_LIBRARY_PATH" environment variable does not work. I suspect Linker can't find C library, ignores .rpath? #489 might be related as well but I have yet to test.
  • To fix this we can reuse the sys_newfstat implementation, which returns the correct struct stat structure to userland

newfstat

  • Currently sys_newfstat returns a structure based on the x86_64 version of struct stat ref1
  • This does not account for the different struct stat definition on 32-bit x86 ref1 (Notice the #ifdef)

Boyan-MILANOV
Boyan-MILANOV approved these changes May 4, 2022
View reviewed changes
Copy link
Contributor

@Boyan-MILANOV Boyan-MILANOV left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM!

@ekilmer ekilmer merged commit 2d0be73 into trailofbits:master May 4, 2022
@lordidiot lordidiot deleted the stat-syscall branch May 5, 2022 04:16
ekilmer added a commit that referenced this pull request May 16, 2022
@ekilmer
Merge branch 'master' into capstone-5-dev
7a45dbc 
* master:
  Add syscall implementation for `sys_newfstatat` (#2545)
  Fix newstat and newfstat (#2544)
ekilmer added a commit that referenced this pull request May 23, 2022
@ekilmer
Merge branch 'master' into chess
9c3f447 
* master:
  Capstone 5 updates (5.0.0rc2) (#1701)
  Add syscall implementation for `sys_newfstatat` (#2545)
  Fix newstat and newfstat (#2544)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@Boyan-MILANOV Boyan-MILANOV Boyan-MILANOV approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@lordidiot @Boyan-MILANOV @ekilmer