Bug 15559 - open(O_CREAT|O_EXCL) erroneously resolves symlinks
Summary: open(O_CREAT|O_EXCL) erroneously resolves symlinks
Status: RESOLVED FIXED
Alias: None
Product: Pseudo
Classification: Yocto Project Subprojects
Component: pseudo (show other bugs)
Version: master
Hardware: x86 Multiple
: Medium+ normal
Target Milestone: 5.1 M3
Assignee: Mark Hatle
QA Contact:
URL:
Whiteboard:
Depends on:
Blocks:
 
Reported: 2024-07-24 12:36 UTC by Simon Lindholm
Modified: 2024-07-25 19:25 UTC (History)
5 users (show)

See Also:
OS type for building Yocto: ---
Type of Regression: ---
Verified:
Documentation change: No (bug/feature does not impact docs)


Attachments
Patch for the bug, with some test cases. (7.11 KB, patch)
2024-07-24 12:36 UTC, Simon Lindholm
no flags Details | Diff
Revised patch (9.36 KB, patch)
2024-07-25 19:24 UTC, Mark Hatle
no flags Details | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Simon Lindholm 2024-07-24 12:36:25 UTC
Created attachment 5060 [details]
Patch for the bug, with some test cases.

In Linux, in a call to open(pathname, O_CREAT|O_EXCL, ...), if pathname exists and is a symlink, pseudo will resolve the symlink instead immediately setting errno to EEXIST - which is what will happen when running without psuedo (i.e. O_CREAT|O_EXCL implies O_NOFOLLOW).

For instance, consider the following snippet:

$ cat -n pseudo_test.sh 
     1 #!/bin/sh -e
     2 
     3 ln -s noexist.txt some_link
     4 
     5 py_code="""
     6 import os
     7 try:
     8     os.open('some_link', os.O_RDWR | os.O_CREAT | os.O_EXCL)
     9     print('Success')
    10 except OSError as e:
    11     print(os.strerror(e.errno))
    12 """
    13 
    14 echo -n "Without psuedo: " && python3 -c "$py_code"
    15 echo -n "With psuedo: " && pseudo python3 -c "$py_code"

$ ./pseudo_test.sh 
Without psuedo: File exists
With psuedo: Success

Issues from this could potentially manifest itself in various different ways, but this bug was discovered while investigating problems with a bitbake recipe that had a fakeroot-task in which it extracted a tar file containing symlinks. The first time the task was run it worked as expected, but when run again, tar ran into problems. When extracting files from a tar-archive, GNU tar will first try open(O_CREAT|O_EXCL), and if that doesn't succeed, check errno and if it's EEXIST it will remove the file, but if it gets e.g. ENOENT or EACCES (which would happen with some symlinks when they were resolved), then it will consider that an unrecoverable error.

I'm attaching a patch to fix this bug (with some added test cases).

The patch changes the flags for the different open calls (open, openat, open64, etc.) in ports/linux/wrapfuncs.in from flags=flags&O_NOFOLLOW to flags=flags&(O_NOFOLLOW|O_EXCL). 

In the generated pseudo_wrapfuncs.c the 'flags' from wrapfuncs.in are used for the 'leave_last' argument to pseudo_root_path(), i.e. by changing from just O_NOFOLLOW to O_NOFOLLOW|O_EXCL, 'leave_last' will be non-zero if either O_NOFOLLOW or O_EXCL is set (i.e. don't resolve symlink if O_EXCL is set).

The patch is for master, but the bug is also present in release 1.9.0 (and probably earlier).
Comment 1 Mark Hatle 2024-07-25 18:23:24 UTC
Reviewed the proposed change.  Consulting the open(2) man page, it indicates that O_EXCL behavior is 'undefined' unless used with O_CREAT.  When both O_EXCL and O_CREAT are specified symbolic links are NOT followed.  (O_NOFOLLOW behavior).  There is an exception to block devices, but I don't believe that applies in this case.

Additionally looking through the Linux 6.6.30 sources, I see that Linux assumes that
O_CREAT|O_EXCL implies O_NOFOLLOW.  (fs/open.c: build_open_flags(...))

So based on these two references, I think a fully "correct/complete" solution would set the flag if O_NOFOLLOW or (O_CREAT and O_EXCL) are set.   But I'm not sure that sort of logic operation is possible in the wrappers.  I'm still investigating this.
Comment 2 Mark Hatle 2024-07-25 19:24:00 UTC
Created attachment 5063 [details]
Revised patch

Patch was revised from original version to make the check more specific to how the Linux kernel handles the flags.
Comment 3 Mark Hatle 2024-07-25 19:25:36 UTC
Revised patch has been sent to the yocto-patches list for inclusion.