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

fix rpmbuild failure because of wrong symlink length on some filesystems #1740

Closed

Conversation

skysley
Copy link

@skysley skysley commented Jun 30, 2021

Reference Issues

#1682

What does this fix?

The man page of stat() mentions that for symbolic links the length of the pointed-to path is stored in stat.st_size. However, as reported here and here on some filesystems stat.st_size does not equal the length of the path that the symbolic link points to (e.g., in an fscrypt-encrypted directory).

Although this is not strictly a bug in rpmbuild, it would be nice to circumvent this problem in general. To this end, this patch tests for symbolic links and explicitly retrieves the length of the pointed-to path. This value is then stored in stat.st_size.

@ffesti
Copy link
Contributor

ffesti commented Jun 30, 2021

Thanks for this very thorough analysis and bug report!

But I am hesitant to merge this just to work around what looks to me like a very obvious bug in an file system. Let's keep this open until we see where google/fscrypt#305 is going. But I suspect this issue is going to get fixed (hopefully) quickly.

The behaviour of stat is so deeply part of POSIX that I am not willing to make compromises there that easily.

@ffesti ffesti added the DONT DO NOT merge, for whatever reason label Jun 30, 2021
@DemiMarie
Copy link
Contributor

Also note that PATH_MAX is not a good value for sizing stack buffers: on some systems (ex: GNU/Hurd) it is an absurdly large value. Paths should be allocated on the heap instead.

@DemiMarie
Copy link
Contributor

But I am hesitant to merge this just to work around what looks to me like a very obvious bug in an file system. Let's keep this open until we see where google/fscrypt#305 is going. But I suspect this issue is going to get fixed (hopefully) quickly.

As mentioned in that issue, returning an accurate value would cause a performance penalty, which is why it has not been done yet. Since rpmbuild (presumably) needs the actual symlink target, it can just call readlink and cache the result, which has no performance penalty.

@skysley
Copy link
Author

skysley commented Jul 1, 2021

Thank you for the responses! I understand the problem(s) of this pull request.

I have two questions:

  1. Why is the archive size calculated in advance in the first place? Is this some kind of sanity check or is it required somewhere?
  2. Would it be possible, in case the size check fails, to give some better error message? Currently it is giving "write failed - No such file or directory" which made me wonder: "Which file or directory? What did I do wrong?".

@ffesti
Copy link
Contributor

ffesti commented Aug 17, 2021

OK, it looks like this is going to get fixed in the kernel. Closing this here. In case the kernel fix won't make feel free to re-open the discussion.

@ffesti ffesti closed this Aug 17, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
DONT DO NOT merge, for whatever reason
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants