Skip to content

When opening broken symlink with O_CREAT, create file at target #23002

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

Merged
merged 12 commits into from
Dec 5, 2024

Conversation

hoodmane
Copy link
Collaborator

@hoodmane hoodmane commented Nov 25, 2024

If we open a broken symlink with O_CREAT we should create a new file as the target of the symlink.
This resolves #23001.

To fix the problem, I added an extra handleBrokenLink option to lookupPath. When this option is passed, if the input path is a broken symlink, instead of raising EEXIST, it returns { path: theTarget, node: undefined}. Then in open we update path based on the returned path, so if the node doesn't exist we attempt create it at the resolved target path of the symlink rather than trying to create it on the source of the symlink which raises EEXIST again.

  • Add a test

@hoodmane hoodmane marked this pull request as draft November 25, 2024 15:18
@hoodmane

This comment was marked as resolved.

@hoodmane hoodmane changed the title When opening broken symlink with O_CREAT, replace it When opening broken symlink with O_CREAT, create file at target Nov 25, 2024
@hoodmane hoodmane force-pushed the open-broken-link-create branch from 55152ce to 5bd422e Compare November 25, 2024 19:20
@hoodmane hoodmane marked this pull request as ready for review November 25, 2024 19:20
Comment on lines 223 to 226
if (!current.node_ops.readlink) {
throw new FS.ErrnoError({{{ cDefs.ENOSYS }}});
}
var link = current.node_ops.readlink(current);
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Before this change, FS.readlink calls lookupPath again which repeats exactly the same work we've done up to this point in this function except with opts.follow false. Rather than doing that, we already have the node here so I figure it's better just to call node_ops.readlink.

@hoodmane
Copy link
Collaborator Author

hoodmane commented Dec 5, 2024

I think this one is ready to merge?

@sbc100 sbc100 merged commit c60dcfe into emscripten-core:main Dec 5, 2024
25 of 28 checks passed
@sbc100
Copy link
Collaborator

sbc100 commented Dec 10, 2024

Looks like there is an asan failure with this test: https://logs.chromium.org/logs/emscripten-releases/buildbucket/cr-buildbucket/8728915358824971169/+/u/Emscripten_testsuite__ASan_/stdout

-- begin program output --
link result: 0
source_fd: 3, errno: 0 No error information
target_fd: 3, errno: 0 No error information
=================================================================
==42==ERROR: AddressSanitizer: stack-buffer-overflow on address 0x1295703a at pc 0x00002a60 bp 0x12956d90 sp 0x12956d9c
READ of size 1 at 0x1295703a thread T0
    #0 0x2a60 in memchr+0x2a60 (/b/s/w/ir/x/t/emtest_6jfnlc89/emscripten_test_asan_ap09sbet/test_unistd_write_broken_link.js+0x2a60)
    #1 0x2a78 in strnlen+0x2a78 (/b/s/w/ir/x/t/emtest_6jfnlc89/emscripten_test_asan_ap09sbet/test_unistd_write_broken_link.js+0x2a78)
    #2 0x40f4 in printf_core+0x40f4 (/b/s/w/ir/x/t/emtest_6jfnlc89/emscripten_test_asan_ap09sbet/test_unistd_write_broken_link.js+0x40f4)
    #3 0x3312 in __vfprintf_internal+0x3312 (/b/s/w/ir/x/t/emtest_6jfnlc89/emscripten_test_asan_ap09sbet/test_unistd_write_broken_link.js+0x3312)
    #4 0x5334 in vfprintf+0x5334 (/b/s/w/ir/x/t/emtest_6jfnlc89/emscripten_test_asan_ap09sbet/test_unistd_write_broken_link.js+0x5334)
    #5 0x1fb5 in printf+0x1fb5 (/b/s/w/ir/x/t/emtest_6jfnlc89/emscripten_test_asan_ap09sbet/test_unistd_write_broken_link.js+0x1fb5)
    #6 0x1393 in __original_main+0x1393 (/b/s/w/ir/x/t/emtest_6jfnlc89/emscripten_test_asan_ap09sbet/test_unistd_write_broken_link.js+0x1393)
    #7 0x1812 in main+0x1812 (/b/s/w/ir/x/t/emtest_6jfnlc89/emscripten_test_asan_ap09sbet/test_unistd_write_broken_link.js+0x1812)
    #8 0x80000299  (JavaScript+0x299)
    #9 0x80001456 in callMain /b/s/w/ir/x/t/emtest_6jfnlc89/emscripten_test_asan_ap09sbet/test_unistd_write_broken_link.js:5206:15
    #10 0x8000147c in doRun /b/s/w/ir/x/t/emtest_6jfnlc89/emscripten_test_asan_ap09sbet/test_unistd_write_broken_link.js:5244:23
    #11 0x80001486 in run /b/s/w/ir/x/t/emtest_6jfnlc89/emscripten_test_asan_ap09sbet/test_unistd_write_broken_link.js:5254:5
    #12 0x8000144a in runCaller /b/s/w/ir/x/t/emtest_6jfnlc89/emscripten_test_asan_ap09sbet/test_unistd_write_broken_link.js:5194:19
    #13 0x80000261 in removeRunDependency /b/s/w/ir/x/t/emtest_6jfnlc89/emscripten_test_asan_ap09sbet/test_unistd_write_broken_link.js:609:7
    #14 0x800003c8 in receiveInstance /b/s/w/ir/x/t/emtest_6jfnlc89/emscripten_test_asan_ap09sbet/test_unistd_write_broken_link.js:968:5
    #15 0x800003d9 in receiveInstantiationResult /b/s/w/ir/x/t/emtest_6jfnlc89/emscripten_test_asan_ap09sbet/test_unistd_write_broken_link.js:985:5

Address 0x1295703a is located in stack of thread T0 at offset 26 in frame
    #0 0x11 in legalimport$__wasi_clock_time_get+0x11 (/b/s/w/ir/x/t/emtest_6jfnlc89/emscripten_test_asan_ap09sbet/test_unistd_write_broken_link.js+0x11)

  This frame has 2 object(s):
    [16, 26) 'buf' <== Memory access at offset 26 overflows this variable
    [48, 58) 'buf24'
HINT: this may be a false positive if your program uses some custom stack unwind mechanism, swapcontext or vfork
      (longjmp and C++ exceptions *are* supported)
SUMMARY: AddressSanitizer: stack-buffer-overflow (/b/s/w/ir/x/t/emtest_6jfnlc89/emscripten_test_asan_ap09sbet/test_unistd_write_broken_link.js+0x2a5c) in memchr+0x2a5c
Shadow bytes around the buggy address:
  0x12956d80: 00 00 00 00 f1 f1 00 f2 f2 f2 00 00 00 f2 f2 f2
  0x12956e00: f2 f2 00 f2 f2 f2 04 f3 00 00 00 00 f1 f1 04 f2
  0x12956e80: 04 f2 00 00 00 00 00 f2 f2 f2 f2 f2 00 00 00 00
  0x12956f00: 00 00 00 00 00 00 f2 f2 f2 f2 00 00 00 00 00 00
  0x12956f80: 00 00 00 00 f3 f3 f3 f3 00 00 00 00 f1 f1 04 f3
=>0x12957000: 00 00 00 00 f1 f1 00[02]f2 f2 f8 f8 f3 f3 00 00
  0x12957080: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
  0x12957100: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
  0x12957180: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
  0x12957200: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
  0x12957280: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
Shadow byte legend (one shadow byte represents 8 application bytes):
  Addressable:           00
  Partially addressable: 01 02 03 04 05 06 07 
  Heap left redzone:       fa
  Freed heap region:       fd
  Stack left redzone:      f1
  Stack mid redzone:       f2
  Stack right redzone:     f3
  Stack after return:      f5
  Stack use after scope:   f8
  Global redzone:          f9
  Global init order:       f6
  Poisoned by user:        f7
  Container overflow:      fc
  Array cookie:            ac
  Intra object redzone:    bb
  ASan internal:           fe
  Left alloca redzone:     ca
  Right alloca redzone:    cb
==42==ABORTING

hedwigz pushed a commit to hedwigz/emscripten that referenced this pull request Dec 18, 2024
…ripten-core#23002)

If we open a broken symlink with `O_CREAT` we should create a new file
as the target of the symlink.

Fixes: emscripten-core#23001.
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.

Opening broken symlink with O_CREAT returns EEXIST unlike linux
2 participants