Bug 1311682 - Problem with pipes implemented using unix sockets
Problem with pipes implemented using unix sockets
Status: CLOSED CANTFIX
Product: Red Hat Enterprise Linux 7
Classification: Red Hat
Component: ksh (Show other bugs)
7.2
All All
medium Severity medium
: rc
: ---
Assigned To: Siteshwar Vashisht
BaseOS QE - Apps
:
Depends On:
Blocks: 1298243
  Show dependency treegraph
 
Reported: 2016-02-24 12:56 EST by Paulo Andrade
Modified: 2017-01-19 07:52 EST (History)
3 users (show)

See Also:
Fixed In Version:
Doc Type: If docs needed, set a value
Doc Text:
Story Points: ---
Clone Of:
Environment:
Last Closed: 2017-01-19 07:52:01 EST
Type: Bug
Regression: ---
Mount Type: ---
Documentation: ---
CRM:
Verified Versions:
Category: ---
oVirt Team: ---
RHEL 7.3 requirements from Atomic Host:
Cloudforms Team: ---


Attachments (Terms of Use)
Minimal Reproducer (605 bytes, text/x-csrc)
2017-01-18 08:50 EST, Siteshwar Vashisht
no flags Details

  None (edit)
Description Paulo Andrade 2016-02-24 12:56:35 EST
The test case is very simple:

$ cat /etc/passwd | head -1 /dev/stdin
head: cannot open ‘/dev/stdin’ for reading: No such device or address

while it should show something like:

$ cat /etc/passwd | head -1 /dev/stdin
root:x:0:0:root:/root:/bin/bash

  The alpha and beta branches of https://github.com/att/ast
do not have the problem, but the master branch has the
problem.

  One quick hack is to "#if 0" the chunk in
src/cmd/ksh93/sh/io.c:

#      if _socketpair_shutdown_mode
#         define pipe(v) ((socketpair(AF_UNIX,SOCK_STREAM,0,v)<0||shutdown((v)[1],SHUT_RD)<0||fchmod((v)[1],S_IWUSR)<0||shutdown((v)[0],SHUT_WR)<0||fchmod((v)[0],S_IRUSR)<0)?(-1):0)
#      else
#         define pipe(v) ((socketpair(AF_UNIX,SOCK_STREAM,0,v)<0||shutdown((v)[1],SHUT_RD)<0||shutdown((v)[0],SHUT_WR)<0)?(-1):0)
#      endif

so that it does not redefine the "pipe" identifier;
or change src/cmd/ksh93/features/poll to fail all
tests, and not define _pipe_socketpair.

  But that causes some test errors, for example:
+	io.sh[254]: <# not working for pipes
+	io.sh[339]: read -n3 from pipe not working
+	io.sh[348]: read -n3 from fifo failed -- expected 'a', got 'abc'
+	io.sh[351]: read -n1 from fifo failed -- expected 'b', got 'd'
and causes file descriptor leaks:
+	subshell.sh[578]: file descriptor 11 not close on exec

  The likely commit that corrects the issue is a 280k lines
commit/merge, and is not trivial. Basically it does a significantly
different way of handling descriptors, mostly related to close-on-exec.

  I tried an adaptation of the core idea, but that is too few code,
and likely would require a quite large patch, not viable for
rhel6 and rhel7. The core idea would be to patch sh_pipe like
this:

"""
--- ksh-20120801/src/cmd/ksh93/sh/io.c.orig	2016-02-24 13:54:09.443932505 -0300
+++ ksh-20120801/src/cmd/ksh93/sh/io.c	2016-02-24 13:59:32.807470766 -0300
@@ -893,8 +893,12 @@ int	sh_pipe(register int pv[])
 		errormsg(SH_DICT,ERROR_system(1),e_pipe);
 	pv[0] = sh_iomovefd(pv[0]);
 	pv[1] = sh_iomovefd(pv[1]);
-	shp->fdstatus[pv[0]] = IONOSEEK|IOREAD;
-	shp->fdstatus[pv[1]] = IONOSEEK|IOWRITE;
+	if(pv[0]>2)
+		fcntl(pv[0],F_SETFD,FD_CLOEXEC);
+	if(pv[1]>2)
+		fcntl(pv[1],F_SETFD,FD_CLOEXEC);
+	shp->fdstatus[pv[0]] = IONOSEEK|IOREAD|IOCLEX;
+	shp->fdstatus[pv[1]] = IONOSEEK|IOWRITE|IOCLEX;
 	sh_subsavefd(pv[0]);
 	sh_subsavefd(pv[1]);
 	return(0);
"""
but that causes even worse problems, for example:
	shcomp-basic.ksh[345]: process substitution of compound commands not working
cat: /dev/fd/4: No such file or directory
	shcomp-basic.ksh[354]: process substitution not working outside for or while loop

  Interestingly, I recently rediscovered a chunk almost identical
in beta and alpha ksh, that almost matches the attachment at
https://bugzilla.redhat.com/show_bug.cgi?id=1259898
[table of file descriptors not updated on F_DUPFD_CLOEXEC]

  My impression is that using unix sockets, instead of real
pipes (like bash at least) is hiding issues with leak of
file descriptors.

  I believe there should be ongoing plan to update to ksh-2014,
so, please as well, check this test condition.
Comment 5 Siteshwar Vashisht 2017-01-18 08:50 EST
Created attachment 1242166 [details]
Minimal Reproducer

This reproducer mimics what ksh does with stdin when sockets are used instead of pipes.

Output:

$ cc test.c
$ ./a.out
Failed to open stdin: No such device or address
Comment 6 Siteshwar Vashisht 2017-01-18 09:11:07 EST
Paulo,

This seems to be the standard behaviour for unix domain sockets, it can not be opened using filename. It is not related to close-on-exec attribute for file descriptors.

In the beta branch at [1] I can still see a macro to replace pipe() with socketpair(), so I presume the problem still exists in beta branch in github. Can you confirm you compiled with socketpair() instead of pipe() when you were testing alpha and beta branches ?

[1] https://github.com/att/ast/blob/beta/src/cmd/ksh93/sh/io.c#L100
Comment 7 Paulo Andrade 2017-01-18 13:55:25 EST
Hi Siteshwar,

I think it I did not make most of my description clear. What I would expect
is that "forcing" ksh to use pipes instead of socketpair, like other shells,
would not cause build time test regressions.

I do not remember all details as I opened this bug almost one year ago :)
I just tested building the beta branch, and it did configure to use socketpair
by default.
Did build on rhel7 (on fedora it does not build ksh, at least not cleanly).
Comment 8 Siteshwar Vashisht 2017-01-19 07:52:01 EST
Paulo,

Switching to real pipes instead of sockets will cause performance degradation. Also, despite of all test cases passing, there is still a major risk of regression with real pipes. It would be too risky to switch to real pipes instead of sockets.

Closing this as cantfix.

Note You need to log in before you can comment on or make changes to this bug.