php.net |  support |  documentation |  report a bug |  advanced search |  search howto |  statistics |  random bug |  login
Bug #71219 php's configure script incorrectly checks for ttyname_r availability
Submitted: 2015-12-25 18:34 UTC Modified: 2016-10-17 13:26 UTC
Votes:1
Avg. Score:3.0 ± 0.0
Reproduced:0 of 0 (0.0%)
From: atoth at atoth dot sote dot hu Assigned:
Status: Closed Package: *Compile Issues
PHP Version: 5.6.16 OS: Linux (Gentoo Hardened)
Private report: No CVE-ID: None
 [2015-12-25 18:34 UTC] atoth at atoth dot sote dot hu
Description:
------------
While PHP's configure script checks for ttyname_r it uses this code snippet:
return ttyname_r(0, buf, 64) ? 1 : 0;
Although ttyname returns a pointer to the fd's null-terminated pathname or NULL on error - ttyname_r behaves differently: it stores the pathname in a buffer and returns 0 on success or an error number.
http://linux.die.net/man/3/ttyname_r
Therefore the above cited check will report a failure upon success.
Configure emits a messages about posix_ttyname being thread unsafe, despite ttyname_r is available.

I suggest to change the test logic by taking into account the return values. I attach a trivial example patch below.

Please note, that this minor issue affects both 5.6* and the new 7.0* branches. I could not select both branches for the report...


Patches

php-7.0.1-ttyname_r-test.patch (last revision 2015-12-26 17:14 UTC) by atoth at atoth dot sote dot hu)
php-5.6.16-ttyname_r-test.patch (last revision 2015-12-26 17:13 UTC) by atoth at atoth dot sote dot hu)
php-ttyname_r-test.patch (last revision 2015-12-25 18:35 UTC) by atoth at atoth dot sote dot hu)

Add a Patch

Pull Requests

Add a Pull Request

History

AllCommentsChangesGit/SVN commitsRelated reports
 [2015-12-26 05:11 UTC] laruence@php.net
-Status: Open +Status: Feedback
 [2015-12-26 05:11 UTC] laruence@php.net
I don't understant the problem here,

if ttyname_r() return 0; it mean success, then:

 return ttyname_r(0, buf, 64) ? 1 : 0;

return 0, action-if-true is evaluated. seems correct to me.
 [2015-12-26 16:46 UTC] atoth at atoth dot sote dot hu
-Status: Feedback +Status: Open
 [2015-12-26 16:46 UTC] atoth at atoth dot sote dot hu
Your logic is right, however the test still returns false.

"checking for working ttyname_r() implementation... no, posix_ttyname() will be thread-unsafe"

configure.log show this:
"configure:79083: checking for working ttyname_r() implementation
configure:79104: x86_64-pc-linux-gnu-gcc -o conftest -I/usr/include -O2 -march=native -pipe -pthread  -D_REENTRANT -L/usr/lib64 -Wl,-O1 -Wl,--as-needed conftest.c -liodbc -lmcrypt -lltdl -lonig -lstdc++ -lcrypto -lssl -lcrypto -lcrypt -lpam -lgmp -lt1 -lX11 -lXpm -lpng -lz -ljpeg -lvpx -lcrypto -lssl -lcrypto -lenchant -ldb-5.1 -lgdbm -lcurl -lbz2 -lz -lpcre -lcrypto -lssl -lcrypto -lrt -lm -ldl -lnsl  -lxml2 -lz -lm -ldl -lcurl -lnghttp2 -lidn -lrtmp -lz -lgmp -lgnutls -lhogweed -lnettle -lssh2 -lssh2 -lssl3 -lsmime3 -lnss3 -lnssutil3 -lplds4 -lplc4 -lnspr4 -lz -lxml2 -lz -lm -ldl -lfreetype -licui18n -licuuc -licudata -licuio -liodbc -liodbcinst -ldl -lodbc >&5
configure:79104: $? = 0
configure:79104: ./conftest
configure:79104: $? = 1
configure: program exited with status 1
configure: failed program was:
| /* confdefs.h */
| #define PACKAGE_NAME ""
| #define PACKAGE_TARNAME ""
| #define PACKAGE_VERSION ""
--
| #define HAVE_GETPWUID_R 1
| #define HAVE_GETGRGID_R 1
| /* end confdefs.h.  */
|
| #include <unistd.h>
|
| int main(int argc, char *argv[])
| {
|       char buf[64];
|
|       return ttyname_r(0, buf, 64) ? 1 : 0;
| }
|
configure:79114: result: no, posix_ttyname() will be thread-unsafe"

I modified the check to output the return value, which was turned out to be: 25.
Errno #25 means: ENOTTY.

I modified the test program like this:
"
#include <unistd.h>
#include <stdio.h>
#include <errno.h>
#include <string.h>

int main(int argc, char *argv[])
{
        char buf[64];
        int retval;

        retval = ttyname_r(0, buf, 64);
        printf("\nretval 0: %s, fd path 0: %s.\n", strerror(retval), buf);
        retval = ttyname_r(1, buf, 64);
        printf("retval 1: %s, fd path 1: %s.\n", strerror(retval), buf);
        retval = ttyname_r(2, buf, 64);
        printf("retval 2: %s, fd path 2: %s.\n", strerror(retval), buf);

        return ttyname_r(0, buf, 64) ? 1 : 0;
}
"

This is the output I got:
checking for working ttyname_r() implementation...
retval 0: Inappropriate ioctl for device, fd path 0: H1gB?.
retval 1: Success, fd path 1: /dev/pts/2.
retval 2: Inappropriate ioctl for device, fd path 2: /dev/pts/2.

So my logic was wrong, but checking for fd 0 is inappropriate.
I guess ti would be better to check for fd 1, instead!

What's your opinion?
 [2016-07-30 16:25 UTC] nikic@php.net
Automatic comment on behalf of nikic
Revision: http://git.php.net/?p=php-src.git;a=commit;h=14d674442ef86ad4e862228a1ff5ecd322ae7759
Log: Fixed bug #71219
 [2016-07-30 16:25 UTC] nikic@php.net
-Status: Open +Status: Closed
 [2016-07-30 16:36 UTC] nikic@php.net
Automatic comment on behalf of nikic
Revision: http://git.php.net/?p=php-src.git;a=commit;h=c9d3ff0c6e86398b9471428ab49c6a6fa47ae977
Log: Revert &quot;Fixed bug #71219&quot;
 [2016-07-30 16:39 UTC] nikic@php.net
-Status: Closed +Status: Re-Opened
 [2016-07-30 16:39 UTC] nikic@php.net
Sorry, I misunderstood the discussion and applied the incorrect fix. Reverted now.

However the other proposed fix, using fd 1 instead of 0, does not work for me either.

Might it be sufficient to just check for the availability of the ttyname_r symbol?
 [2016-07-31 14:25 UTC] atoth at atoth dot sote dot hu
I believe it's quite possible, that the given file descriptor specified in the fix works well only for Gentoo sandbox. Other environments can have another fd wining.
Therefore the simple configure code snippet would not fit for all circumstances.
One possibility is to test only for the availability of the symbol - as you've suggested. Otherwise the configure code must be more complex for this test.
Please also note I'm just a power user and doesn't want to act like a smartass...
 [2016-10-17 10:10 UTC] bwoebi@php.net
Automatic comment on behalf of nikic
Revision: http://git.php.net/?p=php-src.git;a=commit;h=c9d3ff0c6e86398b9471428ab49c6a6fa47ae977
Log: Revert &quot;Fixed bug #71219&quot;
 [2016-10-17 10:10 UTC] bwoebi@php.net
-Status: Re-Opened +Status: Closed
 [2016-10-17 10:10 UTC] bwoebi@php.net
Automatic comment on behalf of nikic
Revision: http://git.php.net/?p=php-src.git;a=commit;h=14d674442ef86ad4e862228a1ff5ecd322ae7759
Log: Fixed bug #71219
 [2016-10-17 13:26 UTC] nikic@php.net
-Status: Closed +Status: Re-Opened
 [2016-10-17 13:26 UTC] nikic@php.net
Someone messed the repo again, reopening.
 [2017-01-11 08:13 UTC] krakjoe@php.net
Automatic comment on behalf of krakjoe
Revision: http://git.php.net/?p=php-src.git;a=commit;h=51d487786c7ac2393604695318aaa72376161f2f
Log: Fixed bug #71219 configure script incorrectly checks for ttyname_r
 [2017-01-11 08:13 UTC] krakjoe@php.net
-Status: Re-Opened +Status: Closed
 
PHP Copyright © 2001-2017 The PHP Group
All rights reserved.
Last updated: Sun Nov 19 01:31:42 2017 UTC