| Message ID | 20240708151522.2176290-1-andrew.cooper3@citrix.com (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | [for-4.19] tools/libxs: Fix fcntl() invocation in set_cloexec() | expand |
On Mon, Jul 8, 2024 at 4:15 PM Andrew Cooper <andrew.cooper3@citrix.com> wrote: > > set_cloexec() had a bit too much copy&pate from setnonblock(), and > insufficient testing on ancient versions of Linux... > > As written (emulating ancient linux by undef'ing O_CLOEXEC), strace shows: > > open("/dev/xen/xenbus", O_RDWR) = 3 > fcntl(3, F_GETFL) = 0x8002 (flags O_RDWR|O_LARGEFILE) > fcntl(3, 0x8003 /* F_??? */, 0x7ffe4a771d90) = -1 EINVAL (Invalid argument) > close(3) = 0 > > which is obviously nonsense. > > Switch F_GETFL -> F_GETFD, and fix the second invocation to use F_SETFD. With > this, strace is rather happer: > > open("/dev/xen/xenbus", O_RDWR) = 3 > fcntl(3, F_GETFD) = 0 > fcntl(3, F_SETFD, FD_CLOEXEC) = 0 > > Fixes: bf7c1464706a ("tools/libxs: Fix CLOEXEC handling in get_dev()") > Reported-by: Ross Lagerwall <ross.lagerwall@citrix.com> > Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com> Reviewed-by: Ross Lagerwall <ross.lagerwall@citrix.com> Thanks
On 08.07.24 17:15, Andrew Cooper wrote: > set_cloexec() had a bit too much copy&pate from setnonblock(), and > insufficient testing on ancient versions of Linux... > > As written (emulating ancient linux by undef'ing O_CLOEXEC), strace shows: > > open("/dev/xen/xenbus", O_RDWR) = 3 > fcntl(3, F_GETFL) = 0x8002 (flags O_RDWR|O_LARGEFILE) > fcntl(3, 0x8003 /* F_??? */, 0x7ffe4a771d90) = -1 EINVAL (Invalid argument) > close(3) = 0 > > which is obviously nonsense. > > Switch F_GETFL -> F_GETFD, and fix the second invocation to use F_SETFD. With > this, strace is rather happer: > > open("/dev/xen/xenbus", O_RDWR) = 3 > fcntl(3, F_GETFD) = 0 > fcntl(3, F_SETFD, FD_CLOEXEC) = 0 > > Fixes: bf7c1464706a ("tools/libxs: Fix CLOEXEC handling in get_dev()") > Reported-by: Ross Lagerwall <ross.lagerwall@citrix.com> > Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com> Reviewed-by: Juergen Gross <jgross@suse.com> Juergen
diff --git a/tools/libs/store/xs.c b/tools/libs/store/xs.c index c8845b69e284..38a6ce3cf2ea 100644 --- a/tools/libs/store/xs.c +++ b/tools/libs/store/xs.c @@ -182,12 +182,12 @@ static bool setnonblock(int fd, int nonblock) { static bool set_cloexec(int fd) { - int flags = fcntl(fd, F_GETFL); + int flags = fcntl(fd, F_GETFD); if (flags < 0) return false; - return fcntl(fd, flags | FD_CLOEXEC) >= 0; + return fcntl(fd, F_SETFD, flags | FD_CLOEXEC) >= 0; } static int pipe_cloexec(int fds[2])
set_cloexec() had a bit too much copy&pate from setnonblock(), and insufficient testing on ancient versions of Linux... As written (emulating ancient linux by undef'ing O_CLOEXEC), strace shows: open("/dev/xen/xenbus", O_RDWR) = 3 fcntl(3, F_GETFL) = 0x8002 (flags O_RDWR|O_LARGEFILE) fcntl(3, 0x8003 /* F_??? */, 0x7ffe4a771d90) = -1 EINVAL (Invalid argument) close(3) = 0 which is obviously nonsense. Switch F_GETFL -> F_GETFD, and fix the second invocation to use F_SETFD. With this, strace is rather happer: open("/dev/xen/xenbus", O_RDWR) = 3 fcntl(3, F_GETFD) = 0 fcntl(3, F_SETFD, FD_CLOEXEC) = 0 Fixes: bf7c1464706a ("tools/libxs: Fix CLOEXEC handling in get_dev()") Reported-by: Ross Lagerwall <ross.lagerwall@citrix.com> Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com> --- CC: Anthony PERARD <anthony.perard@vates.tech> CC: Juergen Gross <jgross@suse.com> CC: Ross Lagerwall <ross.lagerwall@citrix.com> CC: Oleksii Kurochko <oleksii.kurochko@gmail.com> CC: Jan Beulich <JBeulich@suse.com> I'm embarassed to say that this was only spotted by Ross when I was cherrypicking fixes from staging-4.17 into the XenServer patchqueue. This is urgent to take for 4.19, and to backport into 4.17/18 seeing as the breakage has been backported already. --- tools/libs/store/xs.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)