clisp-devel

readlink bug?

[Sam S. <sd...@gn...>]

Hi Bruno,

I observe the following behavior:

--8<---------------cut here---------------start------------->8---
$ ls -l /proc/1627/fd/
total 0
0 lrwx------ 1 sds sds 64 Mar 14 19:34 0 -> /dev/pts/23
0 l-wx------ 1 sds sds 64 Mar 14 19:34 1 -> /home/sds/src/clisp/current/build-porting64-gcc-standard_heapcodes/z
0 l-wx------ 1 sds sds 64 Mar 14 19:34 2 -> /home/sds/src/clisp/current/build-porting64-gcc-standard_heapcodes/z
0 lr-x------ 1 sds sds 64 Mar 14 19:34 3 -> /home/sds/src/clisp/current/build-porting64-gcc-standard_heapcodes/lisp.run*
0 l-wx------ 1 sds sds 64 Mar 14 19:34 4 -> /home/sds/src/clisp/current/build-porting64-gcc-standard_heapcodes/z
0 l-wx------ 1 sds sds 64 Mar 14 19:34 5 -> /home/sds/src/clisp/current/build-porting64-gcc-standard_heapcodes/z
--8<---------------cut here---------------end--------------->8---

after

GC_SAFE_SYSTEM_CALL(result = readlink(namestring_asciz,linkbuf,linklen));

(gdb) p linkbuf
$5 = "/home/sds/src/clisp/current/build-porting64-gcc-standard_heapcod\n"
(gdb) p namestring_asciz
$6 = 0x7fffffff7ff0 "/proc/1627/fd/1"
(gdb) p fs->fs_stat
$7 = {st_dev = 4, st_ino = 5336534, st_nlink = 1, st_mode = 41152, st_uid = 1001, st_gid = 1001, __pad0 = 0, st_rdev = 0, st_size = 64, st_blksize = 1024, st_blocks = 0, st_atim = {tv_sec = 1489534480, tv_nsec = 542647557}, st_mtim = {tv_sec = 1489534480, tv_nsec = 542647557}, st_ctim = {tv_sec = 1489534480, tv_nsec = 542647557}, __glibc_reserved = {0, 0, 0}}



-- 
Sam Steingold (http://sds.podval.org/) on Ubuntu 16.10 (yakkety) X 11.0.11804000
http://www.childpsy.net/ http://jihadwatch.org http://truepeace.org
http://honestreporting.com http://dhimmi.org http://mideasttruth.com
No snowflake in an avalanche is responsible for anything.

Re: readlink bug?

[Sam S. <sd...@gn...>]

oops, C-c too fast.

basically, linklen == result, but linkbuf is incomplete, and, moreover,
is terminated with a newline(!) and a NULL (against the spec).

> * Sam Steingold <fq...@ta...t> [2017-03-14 20:12:45 -0400]:
>
> I observe the following behavior:
>
> $ ls -l /proc/1627/fd/
> total 0
> 0 lrwx------ 1 sds sds 64 Mar 14 19:34 0 -> /dev/pts/23
> 0 l-wx------ 1 sds sds 64 Mar 14 19:34 1 -> /home/sds/src/clisp/current/build-porting64-gcc-standard_heapcodes/z
> 0 l-wx------ 1 sds sds 64 Mar 14 19:34 2 -> /home/sds/src/clisp/current/build-porting64-gcc-standard_heapcodes/z
> 0 lr-x------ 1 sds sds 64 Mar 14 19:34 3 -> /home/sds/src/clisp/current/build-porting64-gcc-standard_heapcodes/lisp.run*
> 0 l-wx------ 1 sds sds 64 Mar 14 19:34 4 -> /home/sds/src/clisp/current/build-porting64-gcc-standard_heapcodes/z
> 0 l-wx------ 1 sds sds 64 Mar 14 19:34 5 -> /home/sds/src/clisp/current/build-porting64-gcc-standard_heapcodes/z
>
> after
>
> GC_SAFE_SYSTEM_CALL(result = readlink(namestring_asciz,linkbuf,linklen));
>
> (gdb) p linkbuf
> $5 = "/home/sds/src/clisp/current/build-porting64-gcc-standard_heapcod\n"
> (gdb) p namestring_asciz
> $6 = 0x7fffffff7ff0 "/proc/1627/fd/1"
> (gdb) p fs->fs_stat
> $7 = {st_dev = 4, st_ino = 5336534, st_nlink = 1, st_mode = 41152,
> st_uid = 1001, st_gid = 1001, __pad0 = 0, st_rdev = 0, st_size = 64,
> st_blksize = 1024, st_blocks = 0, st_atim = {tv_sec = 1489534480,
> tv_nsec = 542647557}, st_mtim = {tv_sec = 1489534480, tv_nsec =
> 542647557}, st_ctim = {tv_sec = 1489534480, tv_nsec = 542647557},
> __glibc_reserved = {0, 0, 0}}

-- 
Sam Steingold (http://sds.podval.org/) on Ubuntu 16.10 (yakkety) X 11.0.11804000
http://www.childpsy.net/ http://iris.org.il http://jihadwatch.org
http://palestinefacts.org http://ffii.org http://memri.org
Sometimes "pain in the ass" and "headache" are synonyms.

Re: readlink bug?

[Bruno H. <br...@cl...>]

Hi Sam,

> basically, linklen == result, but linkbuf is incomplete, and, moreover,
> is terminated with a newline(!) and a NULL (against the spec).

So, we again need special-case code, like the one you removed on 2011-06-01
(commit 0c6f53e756e4) ?

Bruno

Re: readlink bug?

[Sam S. <sd...@gn...>]

Hi Bruno,

> * Bruno Haible <oe...@py...t> [2017-03-15 07:37:48 +0100]:
>
>> basically, linklen == result, but linkbuf is incomplete, and, moreover,
>> is terminated with a newline(!) and a NULL (against the spec).
>
> So, we again need special-case code, like the one you removed on 2011-06-01
> (commit 0c6f53e756e4) ?

I am stunned by your memory.

No, I fixed it differently.
(https://sourceforge.net/p/clisp/clisp/ci/a3b9fbbbd2dd920535085635584f2ba7afbb362c/)

Not that I am particularly happy with the fix...

-- 
Sam Steingold (http://sds.podval.org/) on darwin Ns 10.3.1504
http://steingoldpsychology.com http://www.childpsy.net http://jij.org
http://www.dhimmitude.org http://iris.org.il http://islamexposedonline.com
Press any key to continue or any other key to quit.

Re: readlink bug?

[Daniel J. <dan...@gm...>]

> So, we again need special-case code, like the one you removed on
2011-06-01 (commit 0c6f53e756e4) ?

Is this happening on every readlink call in the proc directory on such
links?
Then we could write an autoconf test for it to only enable correcting code
when necessary.

Though this kind of fix shouldn't be in CLISP code IMHO. That's what gnulib
is for.

Re: readlink bug?

[Bruno H. <br...@cl...>]

Hi Daniel,

> Then we could write an autoconf test for it to only enable correcting code
> when necessary.

Autoconf tests are a good idea in general. But here there's a problem: The
/proc file system can be mounted or unmounted. In chroot environments it is
normally unmounted. Therefore it is possible that on a build machine of a
distro, there will be no /proc file system, whereas on the machine of the
end user, there will be.

Bruno

Re: readlink bug?

[Sam S. <sd...@gn...>]

Hi Bruno,

> * Bruno Haible <oe...@py...t> [2017-03-15 20:37:51 +0100]:
>
>> Then we could write an autoconf test for it to only enable correcting
>> code when necessary.
>
> Autoconf tests are a good idea in general. But here there's a problem:
> The /proc file system can be mounted or unmounted. In chroot
> environments it is normally unmounted. Therefore it is possible that
> on a build machine of a distro, there will be no /proc file system,
> whereas on the machine of the end user, there will be.

I think this is a general risk that the build machine can be different
in some unknown way from the user box.

An egregious example would be the build machine be free of the [Pentium
FP bug](https://en.wikipedia.org/wiki/Pentium_FDIV_bug) thus building
the Linux kernel without the workaround.

However, Daniel's point is, IMO, quite solid.
The workaround should be in gnulib, not CLISP sources.
We should not have to think about this.

Ideally gnulib should provide

        char *gnulib_readlink(char* path);

which will malloc() it's return value (which the caller will free()).
I don't think the cost of malloc/free is on par with readlink which
requires disk access.

-- 
Sam Steingold (http://sds.podval.org/) on darwin Ns 10.3.1504
http://steingoldpsychology.com http://www.childpsy.net http://mideasttruth.com
http://americancensorship.org http://no2bds.org https://jihadwatch.org
Live free or die.

Re: readlink bug?

[Daniel J. <dan...@gm...>]

Sam wrote:

> However, Daniel's point is, IMO, quite solid.  The workaround should
> be in gnulib, not CLISP sources.  We should not have to think about
> this.
>
> Ideally gnulib should provide
>
>        char *gnulib_readlink(char* path);
>
> which will malloc() it's return value (which the caller will free()).
> I don't think the cost of malloc/free is on par with readlink which
> requires disk access.

Actually there seems to be already a gnulib module providing this:

https://www.gnu.org/software/gnulib/MODULES.html#module=areadlink

Though I doubt that gnulib is correcting the issues we have with
/proc here yet. Above module depends on the readlink module:

https://www.gnu.org/software/gnulib/MODULES.html#module=readlink

This is correcting possible trailing slashes on Solaris, but does not
mention /proc in any way. Perhaps we could add the "fix" (retrying
with a bigger buffer) and only include it on specific hosts?

Btw, which system is this appearing on? Only in /proc, right? Wouldn't
it make more sense to retry with a bigger buffer only when the path is
in /proc?

Re: readlink bug?

[Sam S. <sd...@gn...>]

> * Daniel Jour <qnavry.bregjvt@tznvy.pbz> [2017-03-15 21:25:52 +0000]:
>
> Btw, which system is this appearing on? Only in /proc, right? Wouldn't
> it make more sense to retry with a bigger buffer only when the path is
> in /proc?

Yes, but all of this should be in gnulib.
I hope Bruno will sort it out - he is the portability guru here (inter alia)

-- 
Sam Steingold (http://sds.podval.org/) on darwin Ns 10.3.1504
http://steingoldpsychology.com http://www.childpsy.net
http://memri.org http://honestreporting.com http://mideasttruth.com
Don't take life too seriously, you'll never get out of it alive!