On 12/30/2019 6:09 PM, Brian Inglis wrote: > On 2019-12-30 14:47, Ken Brown wrote: >> On 12/30/2019 3:55 PM, Brian Inglis wrote: >>> On 2019-12-30 12:53, Ken Brown wrote: >>>> On 12/30/2019 2:18 PM, Brian Inglis wrote: >>>>> On 2019-12-29 10:56, Ken Brown wrote: >>>>>> Currently, opening a symlink with O_NOFOLLOW fails with ELOOP. >>>>>> Following Linux, the first patch in this series allows the call to >>>>>> succeed if O_PATH is also specified. >>>>>> >>>>>> According to the Linux man page for 'open', the file descriptor >>>>>> returned by the call should be usable as the dirfd argument in calls >>>>>> to fstatat and readlinkat with an empty pathname, to have >>>>>> the calls operate on the symbolic link. The second and third patches >>>>>> achieve this. For fstatat, we do this by adding support >>>>>> for the AT_EMPTY_PATH flag. >>>>>> >>>>>> Note: The man page mentions fchownat and linkat also. linkat already >>>>>> supports the AT_EMPTY_PATH flag, so nothing needs to be done. But I >>>>>> don't understand how this could work for fchownat, because fchown >>>>>> fails with EBADF if its fd argument was opened with O_PATH. So I >>>>>> haven't touched fchownat. >>>>>> >>>>>> Am I missing something? >>>>> >>>>> WSL $ man 2 chown >>>>> ... >>>>> "AT_EMPTY_PATH (since Linux 2.6.39) >>>>> If pathname is an empty string, operate on the file referred to >>>>> by dirfd (which may have been obtained using the open(2) O_PATH >>>>> flag). In this case, dirfd can refer to any type of file, not >>>>> just a directory. If dirfd is AT_FDCWD, the call operates on >>>>> the current working directory. This flag is Linux-specific; de‐ >>>>> fine _GNU_SOURCE to obtain its definition." >>>>> >>>>> says chown the dirfd, regardless of what it is, >>>>> except if AT_FDCWD, chown the CWD. >>>>> >>>>> WSL $ man 2 open >>>>> "O_PATH (since Linux 2.6.39) >>>>> Obtain a file descriptor that can be used for two purposes: to >>>>> indicate a location in the filesystem tree and to perform >>>>> operations that act purely at the file descriptor level. The >>>>> file itself is not opened, and other file operations (e.g., >>>>> read(2), write(2), fchmod(2), fchown(2), fgetxattr(2), >>>>> ioctl(2), mmap(2)) fail with the error EBADF." >>>>> >>>>> O_PATH does not open the file, so fchown returns EBADF, >>>>> as it requires an fd of an open file. >>>> >>>> I think you've just confirmed what I already said: If fchownat is called >>>> with >>>> AT_EMPTY_PATH, with an empty pathname, and with dirfd referring to a file >>>> that >>>> was opened with O_PATH, then fchownat will fail with EBADF. >>>> >>>> So for the purposes of this patch series, I don't see the point of adding >>>> support for AT_EMPTY_PATH in fchownat. >>>> >>>> Am I missing something? >>> >>> That is the user's problem: it is their responsibility to pass an fd open >>> for >>> reading or searching, not one opened with O_PATH (on Linux or Cygwin), or >>> AT_FDCWD; it is Cygwin's responsibility to ensure that valid args succeed >>> and >>> invalid args return the expected errno. >> >> Yes, but Cygwin doesn't claim to support the AT_EMPTY_PATH flag except in >> linkat. So there is no expected errno. The only way there would be an >> expected >> errno is if we decide to add support for AT_EMPTY_PATH to fchownat. I'm >> saying >> that I don't see the point in doing that, and I'm asking whether I'm missing >> something. If you think I should add that support, please explain why. > > To allow perms changed on the cwd, directories or files with an open fd, to > avoid race conditions, like the other ...at functions. > I don't get why you don't see those as useful cases.
I think we're mis-communicating. This is a patch series whose purpose is to add support for opening a symlink with O_PATH | O_NOFOLLOW. In that connection I modified readlinkat and fstatat to allow the resulting fd to be used as the dirfd argument in those calls, with an empty pathname. I didn't do the same for fchownat because it seems to me that it would always fail with EBADF in that setting. It's not relevant that AT_EMPTY_PATH might be useful for fchownat in a different setting. That could be the subject of a different patch. If you think it would be useful *in the context of this patch series*, please explain why. Ken