sandbox: add newfstatat & copy_file_range to allowed filesystem syscalls #23179

pull fanquake wants to merge 2 commits into bitcoin:master from fanquake:add_newfstatat_to_syscall_exceptions changing 1 files +27 −25
  1. fanquake commented at 0:44 am on October 5, 2021: member
    Similar to #23178, this is a follow up to #20487, which has broken running the unit tests for some developers. Fix this by adding newfstatat to the list of allowed filesystem related calls.
  2. sandbox: add newfstatat to allowed filesystem syscalls ee08741c9c
  3. fanquake added the label Utils/log/libs on Oct 5, 2021
  4. fanquake requested review from practicalswift on Oct 5, 2021
  5. fanquake requested review from achow101 on Oct 5, 2021
  6. achow101 commented at 1:07 am on October 5, 2021: member

    Closer, but now missing copy_file_range too:

    0ERROR: The syscall "copy_file_range" (syscall number 326) is not allowed by the syscall sandbox in thread "httpworker.2". Please report.
    
  7. sandbox: add copy_file_range to allowed filesystem syscalls 44d77d2213
  8. fanquake commented at 1:17 am on October 5, 2021: member

    Closer, but now missing copy_file_range too:

    Added copy_file_range to the list of allowed file related syscalls. File related syscalls are already enabled for the HTTP worker threads.

  9. achow101 commented at 1:31 am on October 5, 2021: member

    ACK 44d77d2213e6bd2e2f700dd8c3c3f932bc1bcb48

    Tested that this does fix the test issues I was running into.

  10. fanquake renamed this:
    sandbox: add newfstatat to allowed filesystem syscalls
    sandbox: add `newfstatat` & `copy_file_range` to allowed filesystem syscalls
    on Oct 5, 2021
  11. laanwj commented at 6:07 am on October 5, 2021: member
    Code review ACK 44d77d2213e6bd2e2f700dd8c3c3f932bc1bcb48
  12. laanwj commented at 6:32 am on October 5, 2021: member

    Doing a GUIX build with #23179 and #23178 because I’m slightly afraid newfstatat & copy_file_range are new syscalls that need to be defined manually as well.

    If so, do you mind if I cherry-pick this into #23178?

  13. fanquake commented at 6:43 am on October 5, 2021: member

    If so, do you mind if I cherry-pick this into #23178?

    Yes that’s fine.

  14. laanwj commented at 6:49 am on October 5, 2021: member
    Looks like it’s not neccessary, GUIX build passes with those two PRs as-is.
  15. in src/util/syscall_sandbox.cpp:539 in 44d77d2213
    559-        allowed_syscalls.insert(__NR_rmdir);      // delete a directory
    560-        allowed_syscalls.insert(__NR_stat);       // get file status
    561-        allowed_syscalls.insert(__NR_statfs);     // get filesystem statistics
    562-        allowed_syscalls.insert(__NR_statx);      // get file status (extended)
    563-        allowed_syscalls.insert(__NR_unlink);     // delete a name and possibly the file it refers to
    564+        allowed_syscalls.insert(__NR_access);          // check user's permissions for a file
    


    laanwj commented at 7:28 am on October 5, 2021:
    I think it creates somewhat of a merge hotspot to re-align these comments every time something is added/removed.
  16. practicalswift approved
  17. practicalswift commented at 9:11 am on October 5, 2021: contributor

    cr ACK 44d77d2213e6bd2e2f700dd8c3c3f932bc1bcb48

    Thanks for improving the experimental syscall sandbox! :)

  18. MarcoFalke merged this on Oct 5, 2021
  19. MarcoFalke closed this on Oct 5, 2021

  20. fanquake deleted the branch on Oct 5, 2021
  21. sidhujag referenced this in commit 6360e632d2 on Oct 5, 2021
  22. DrahtBot locked this on Oct 30, 2022

github-metadata-mirror

This is a metadata mirror of the GitHub repository bitcoin/bitcoin. This site is not affiliated with GitHub. Content is generated from a GitHub metadata backup.
generated: 2024-10-04 13:12 UTC

This site is hosted by @0xB10C
More mirrored repositories can be found on mirror.b10c.me