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

lgetxattr returns ENOENT instead of ENODATA #100

Closed
tkuosmanen opened this issue Feb 19, 2020 · 3 comments
Closed

lgetxattr returns ENOENT instead of ENODATA #100

tkuosmanen opened this issue Feb 19, 2020 · 3 comments

Comments

@tkuosmanen
Copy link

There is an issue with fetching extended attributes on Ubuntu 16.04 platform. Using a build from
the latest top of tree (54362b7), a simple ls -la of the sandbox root gives No such file or directory error message. Commands like cp -a will return non-zero exit code, because they are unable to get the extended attributes of the source directories.

Terminal 1:

$ mkdir dummy_dir
$ cd dummy_dir/
$ echo dummy > regular_file.txt
$ mkdir regular_dir
$ mkdir new_sandbox
$ sandboxfs $PWD/new_sandbox

Terminal 2:

$ cd dummy_dir/
$ ls -la
ls: new_sandbox: No such file or directory  # <- This is caused by failing lgetxattr call.
total 17
drwxr-xr-x  4 tkuosmanen tkuosmanen 4096 Feb 19 10:50 .
drwxr-xr-x 14 tkuosmanen tkuosmanen 4096 Feb 19 10:49 ..
dr-xr-xr-x  2 tkuosmanen tkuosmanen    2 Feb 19 10:58 new_sandbox
drwxr-xr-x  2 tkuosmanen tkuosmanen 4096 Feb 19 10:49 regular_dir
-rw-r--r--  1 tkuosmanen tkuosmanen    6 Feb 19 10:49 regular_file.txt

$ sudo strace -u tkuosmanen ls -la
...
lstat("new_sandbox", {st_mode=S_IFDIR|0555, st_size=2, ...}) = 0
lgetxattr("new_sandbox", "security.selinux", 0x19b4d90, 255) = -1 ENOENT (No such file or directory)
write(2, "ls: ", 4ls: )                     = 4
write(2, "new_sandbox", 11new_sandbox)             = 11
write(2, ": No such file or directory", 27: No such file or directory) = 27
write(2, "\n", 1)                       = 1
...
lstat("regular_dir", {st_mode=S_IFDIR|0755, st_size=4096, ...}) = 0
lgetxattr("regular_dir", "security.selinux", 0x19b7e30, 255) = -1 ENODATA (No data available)
getxattr("regular_dir", "system.posix_acl_access", NULL, 0) = -1 ENODATA (No data available)
getxattr("regular_dir", "system.posix_acl_default", NULL, 0) = -1 ENODATA (No data available)
...
lstat("regular_file.txt", {st_mode=S_IFREG|0644, st_size=6, ...}) = 0
lgetxattr("regular_file.txt", "security.selinux", 0x19b7e90, 255) = -1 ENODATA (No data available)
getxattr("regular_file.txt", "system.posix_acl_access", NULL, 0) = -1 ENODATA (No data available)

I'm able to repro the issue on 4.4.0-133-generic and 4.4.0-171-generic kernels.

@jmmv
Copy link
Contributor

jmmv commented Feb 25, 2020

Huh, so this is interesting.

First, getxattr is indeed returning an incorrect error on a missing extended attribute. But it's not ENOENT: it's ENODATA in all cases, which is correct for Linux but is incorrect for macOS (should be ENOATTR there).

Second, the problem you are observing really comes from a missing file, not a missing attribute. Note that the sandbox you mounted is completely "unmapped": there is nothing in the root directory, so when we try getxattr, we detect that and return ENOENT for the missing underlying path. Which seems wrong. Should probably be returning empty attributes for an unmapped entry.

jmmv added a commit to jmmv/sandboxfs that referenced this issue Feb 25, 2020
On a call to getxattr for an unknown attribute, Linux wants ENODATA
and macOS wants ENOATTR.

Does not fix bazelbuild#100 but discovered through it.
jmmv added a commit to jmmv/sandboxfs that referenced this issue Feb 25, 2020
Instead of returning errors when we deal with a node that has no underlying
path (which happens for scaffold nodes and for deleted nodes), stub out the
return values.  For read operations, pretend the xattrs are not there, and
for write operations, deny the change.

Fixes bazelbuild#100.
@tkuosmanen
Copy link
Author

I originally discovered an xattr issue when doing cp -a as part of a complicated build. I thought I had a minimal repro with the unmapped/empty sandbox, but it seems to be a different issue with similar symptoms. Even more interestingly, now that I tried 2090e44, the original cp -a issue is fixed :)

jmmv added a commit to jmmv/sandboxfs that referenced this issue Feb 25, 2020
Instead of returning errors when we deal with a node that has no underlying
path (which happens for scaffold nodes and for deleted nodes), stub out the
return values.  For read operations, pretend the xattrs are not there, and
for write operations, deny the change.

Fixes bazelbuild#100.
@jmmv jmmv closed this as completed in 0e89d15 Feb 25, 2020
jmmv added a commit that referenced this issue Feb 25, 2020
Instead of returning errors when we deal with a node that has no underlying
path (which happens for scaffold nodes and for deleted nodes), stub out the
return values.  For read operations, pretend the xattrs are not there, and
for write operations, deny the change.

Fixes #100.
@jmmv
Copy link
Contributor

jmmv commented Feb 25, 2020

Should be fixed now. I've also made extended attributes support conditional and disabled by default because I fear there might be other problems, and I also fear these extra vnops may be detrimental to performance.

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

No branches or pull requests

2 participants