Bug#739752: bug#17010: Bug#739752: coreutils: ln segfaults when run with --relative and an empty target

2014-03-14 Thread Pádraig Brady 
On 03/14/2014 03:44 AM, Jim Meyering wrote:
 On Thu, Mar 13, 2014 at 7:22 PM, Pádraig Brady p...@draigbrady.com wrote:

 Interesting. So canonicalize_filename_mode() can fail in this case,
 even with CAN_MISSING. It's unexpected that c_f_m() sets errno=ENOENT
 when CAN_MISSING is set. I wonder should we change that instead
 in gnulib? With CAN_MISSING I would expect this function to work
 on arbitrary strings, including the empty string.
 
 What would you have c_f_m(, CAN_MISSING) return?
 I know of no absolute name corresponding to the dot-relative empty string.

Since with CAN_MISSING we should be just degenerating to string processing,
I would think it slightly better to return xstrdup(),
to avoid special casing that in each caller.

I also notice that c_f_m() can return ENOMEM (from areadlink_with_size).
It's arguable that we should xalloc_die() within c_f_m()
for consistency in that case.

However I also notice that c_f_m(, CAN_MISSING) can return ENOENT
if the current working dir is unlinked.  I'm not sure
there is anything else we could do within c_f_m() to handle that.

Hence since c_f_m() can validly fail even with CAN_MISSING,
I agree your patch is correct.

Please push.

thanks!
Pádraig.


-- 
To UNSUBSCRIBE, email to debian-bugs-dist-requ...@lists.debian.org
with a subject of unsubscribe. Trouble? Contact listmas...@lists.debian.org

Bug#739752: bug#17010: Bug#739752: coreutils: ln segfaults when run with --relative and an empty target

2014-03-14 Thread Jim Meyering 
On Fri, Mar 14, 2014 at 4:49 AM, Pádraig Brady p...@draigbrady.com wrote:
...
 Hence since c_f_m() can validly fail even with CAN_MISSING,
 I agree your patch is correct.

 Please push.

Done.


--
To UNSUBSCRIBE, email to debian-bugs-dist-requ...@lists.debian.org
with a subject of unsubscribe. Trouble? Contact listmas...@lists.debian.org

Bug#739752: bug#17010: Bug#739752: coreutils: ln segfaults when run with --relative and an empty target

2014-03-14 Thread Jim Meyering 
On Fri, Mar 14, 2014 at 9:27 AM, Jim Meyering j...@meyering.net wrote:
 On Fri, Mar 14, 2014 at 4:49 AM, Pádraig Brady p...@draigbrady.com wrote:
 ...
 Hence since c_f_m() can validly fail even with CAN_MISSING,
 I agree your patch is correct.

 Please push.

 Done.

Hmm... For the record, I also pushed (accidentally) two additional changes that
had been languishing in the affected repository.  Here they are:
From 42d2377b813507d50f5a67076bb20134ceb2fc10 Mon Sep 17 00:00:00 2001
From: Jim Meyering j...@meyering.net
Date: Mon, 5 Aug 2013 07:28:34 -0700
Subject: [PATCH 1/2] scripts: autotools-install: update

* scripts/autotools-install: Update version numbers of latest
automake and gettext packages.
---
 scripts/autotools-install | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/scripts/autotools-install b/scripts/autotools-install
index 403644f..c6a8e36 100755
--- a/scripts/autotools-install
+++ b/scripts/autotools-install
@@ -24,9 +24,9 @@ tarballs='
   http://pkgconfig.freedesktop.org/releases/pkg-config-0.28.tar.gz
   http://ftp.gnu.org/gnu/m4/m4-1.4.16.tar.gz
   http://ftp.gnu.org/gnu/autoconf/autoconf-2.69.tar.gz
-  http://ftp.gnu.org/gnu/automake/automake-1.12.6.tar.gz
+  http://ftp.gnu.org/gnu/automake/automake-1.14.tar.gz
   http://ftp.gnu.org/gnu/libtool/libtool-2.4.2.tar.gz
-  http://ftp.gnu.org/gnu/gettext/gettext-0.18.2.tar.gz
+  http://ftp.gnu.org/gnu/gettext/gettext-0.18.3.tar.gz
 '

 usage() {
-- 
1.9.0.167.g384364b


From 4f2118221777dc915f2df4e2ed3b65c68301 Mon Sep 17 00:00:00 2001
From: Jim Meyering meyer...@fb.com
Date: Mon, 18 Nov 2013 10:13:06 -0800
Subject: [PATCH 2/2] maint: avoid attribute-const-suggesting warning from
 gcc

* gl/lib/fadvise.c: Use a pragma to turn off this warning option:
-Wsuggest-attribute=const.  Without this change, building with
--enable-gcc-warnings would evoke this error:

lib/fadvise.c:25:1: error: function might be candidate for\
attribute 'const' [-Werror=suggest-attribute=const]
---
 gl/lib/fadvise.c | 6 ++
 1 file changed, 6 insertions(+)

diff --git a/gl/lib/fadvise.c b/gl/lib/fadvise.c
index 3456ce1..562f1eb 100644
--- a/gl/lib/fadvise.c
+++ b/gl/lib/fadvise.c
@@ -14,6 +14,12 @@
You should have received a copy of the GNU General Public License
along with this program.  If not, see http://www.gnu.org/licenses/.  */

+/* Without this pragma, gcc suggests that (given !HAVE_POSIX_FADVISE)
+   the the fdadvise function might be candidate for attribute 'const'.  */
+#if (__GNUC__ == 4  6 = __GNUC_MINOR__) || 4  __GNUC__
+# pragma GCC diagnostic ignored -Wsuggest-attribute=const
+#endif
+
 #include config.h
 #include fadvise.h

-- 
1.9.0.167.g384364b

Bug#739752: coreutils: ln segfaults when run with --relative and an empty target

2014-03-13 Thread Jim Meyering 
On Sat, Feb 22, 2014 at 1:57 AM, Erik Bernstein e...@fscking.org wrote:
 Package: coreutils
 Version: 8.21-1
 Severity: normal

 Hi,

 when ln is run with --relative --symbolic and and empty string as the
 target, it ungracefully dies with a segmentation fault. The memory
 violation appears to happen in src/relpath.c:38 when the two input paths
 are checked for leading slashes:

   if ((path1[1] == '/') != (path2[1] == '/'))

 How to reproduce:
 [1] Open a terminal
 [2] run: ln -sr '' foobar

 Result: segmentation fault  ln -sr '' foobar
 Expected result: Some kind of error message
...

Thank you for the bug report!
That also affected the very latest code in git.
Here is a patch:
From a6d2db8b6dfe15344aba4aefe9545eb3a4876d45 Mon Sep 17 00:00:00 2001
From: Jim Meyering meyer...@fb.com
Date: Thu, 13 Mar 2014 17:05:04 -0700
Subject: [PATCH] ln: with -sr, don't segfault for a TARGET of ''

Prior to this change, ln -sr '' F would segfault, attempting
to read path2[1] in relpath.c's path_common_prefix function.
This problem arises whenever canonicalize_filename_mode returns
NULL.
* src/ln.c (convert_abs_rel): Call relpath only when
both canonicalize_filename_mode calls return non-NULL.
* tests/ln/relative.sh: Add a test to trigger this failure.
* THANKS.in: List reporter's name/address.
* NEWS (Bug fixes): Mention it.
Reported by Erik Bernstein in 739...@bugs.debian.org.
---
 NEWS |  2 ++
 THANKS.in|  1 +
 src/ln.c | 16 ++--
 tests/ln/relative.sh |  5 +
 4 files changed, 18 insertions(+), 6 deletions(-)

diff --git a/NEWS b/NEWS
index 62966b2..b3ad65c 100644
--- a/NEWS
+++ b/NEWS
@@ -25,6 +25,8 @@ GNU coreutils NEWS-*- 
outline -*-
   it would display an error, requiring --no-dereference to avoid the issue.
   [bug introduced in coreutils-5.3.0]

+  ln -sr '' F no longer segfaults: now it fails with the expected diagnostic
+
   shuf --repeat no longer dumps core if the input is empty.
   [bug introduced with the --repeat feature in coreutils-8.22]

diff --git a/THANKS.in b/THANKS.in
index 561d18c..2670265 100644
--- a/THANKS.in
+++ b/THANKS.in
@@ -193,6 +193,7 @@ Eric G. Miller  e...@jps.net
 Eric Pementepeme...@northpark.edu
 Eric S. Raymond e...@snark.thyrsus.com
 Erik Bennettbenn...@cvo.oneworld.com
+Erik Bernstein  e...@fscking.org
 Erik Corry  e...@kroete2.freinet.de
 Felix Lee   f...@teleport.com
 Felix Rauch Valenti fra...@cse.unsw.edu.au
diff --git a/src/ln.c b/src/ln.c
index aab9cf2..6726699 100644
--- a/src/ln.c
+++ b/src/ln.c
@@ -149,13 +149,17 @@ convert_abs_rel (const char *from, const char *target)
   char *realdest = canonicalize_filename_mode (targetdir, CAN_MISSING);
   char *realfrom = canonicalize_filename_mode (from, CAN_MISSING);

-  /* Write to a PATH_MAX buffer.  */
-  char *relative_from = xmalloc (PATH_MAX);
-
-  if (!relpath (realfrom, realdest, relative_from, PATH_MAX))
+  char *relative_from = NULL;
+  if (realdest  realfrom)
 {
-  free (relative_from);
-  relative_from = NULL;
+  /* Write to a PATH_MAX buffer.  */
+  relative_from = xmalloc (PATH_MAX);
+
+  if (!relpath (realfrom, realdest, relative_from, PATH_MAX))
+{
+  free (relative_from);
+  relative_from = NULL;
+}
 }

   free (targetdir);
diff --git a/tests/ln/relative.sh b/tests/ln/relative.sh
index 7636695..e8c25dc 100755
--- a/tests/ln/relative.sh
+++ b/tests/ln/relative.sh
@@ -45,4 +45,9 @@ mkdir web
 ln -sr latest web/latest
 test $(readlink web/latest) = '../release2' || fail=1

+# Expect this to fail with exit status 1.
+# Prior to coreutils-8.23, it would segfault.
+ln -sr '' F
+test $? = 1 || fail=1
+
 Exit $fail
-- 
1.9.0.167.g384364b

Bug#739752: bug#17010: Bug#739752: coreutils: ln segfaults when run with --relative and an empty target

2014-03-13 Thread Pádraig Brady 
On 03/14/2014 01:42 AM, Jim Meyering wrote:
 From a6d2db8b6dfe15344aba4aefe9545eb3a4876d45 Mon Sep 17 00:00:00 2001
 From: Jim Meyering meyer...@fb.com
 Date: Thu, 13 Mar 2014 17:05:04 -0700
 Subject: [PATCH] ln: with -sr, don't segfault for a TARGET of ''
 
 Prior to this change, ln -sr '' F would segfault, attempting
 to read path2[1] in relpath.c's path_common_prefix function.
 This problem arises whenever canonicalize_filename_mode returns
 NULL.
 * src/ln.c (convert_abs_rel): Call relpath only when
 both canonicalize_filename_mode calls return non-NULL.
 * tests/ln/relative.sh: Add a test to trigger this failure.
 * THANKS.in: List reporter's name/address.
 * NEWS (Bug fixes): Mention it.
 Reported by Erik Bernstein in 739...@bugs.debian.org.

We can amend with the now allocated:

  Fixes http://bugs.gnu.org/17010

 diff --git a/NEWS b/NEWS
 index 62966b2..b3ad65c 100644
 --- a/NEWS
 +++ b/NEWS
 @@ -25,6 +25,8 @@ GNU coreutils NEWS-*- 
 outline -*-
it would display an error, requiring --no-dereference to avoid the issue.
[bug introduced in coreutils-5.3.0]
 
 +  ln -sr '' F no longer segfaults: now it fails with the expected diagnostic

Probably should add:

 [bug introduced with the --relative feature in coreutils-8.16]

 diff --git a/src/ln.c b/src/ln.c
 index aab9cf2..6726699 100644
 --- a/src/ln.c
 +++ b/src/ln.c
 @@ -149,13 +149,17 @@ convert_abs_rel (const char *from, const char *target)
char *realdest = canonicalize_filename_mode (targetdir, CAN_MISSING);
char *realfrom = canonicalize_filename_mode (from, CAN_MISSING);

Interesting. So canonicalize_filename_mode() can fail in this case,
even with CAN_MISSING. It's unexpected that c_f_m() sets errno=ENOENT
when CAN_MISSING is set. I wonder should we change that instead
in gnulib? With CAN_MISSING I would expect this function to work
on arbitrary strings, including the empty string.

 
 -  /* Write to a PATH_MAX buffer.  */
 -  char *relative_from = xmalloc (PATH_MAX);
 -
 -  if (!relpath (realfrom, realdest, relative_from, PATH_MAX))
 +  char *relative_from = NULL;
 +  if (realdest  realfrom)
  {
 -  free (relative_from);
 -  relative_from = NULL;
 +  /* Write to a PATH_MAX buffer.  */
 +  relative_from = xmalloc (PATH_MAX);
 +
 +  if (!relpath (realfrom, realdest, relative_from, PATH_MAX))
 +{
 +  free (relative_from);
 +  relative_from = NULL;
 +}
  }
 
free (targetdir);

 diff --git a/tests/ln/relative.sh b/tests/ln/relative.sh
 +# Expect this to fail with exit status 1.
 +# Prior to coreutils-8.23, it would segfault.
 +ln -sr '' F
 +test $? = 1 || fail=1

Won't the ln succeed on FreeBSD as per:
http://bugs.gnu.org/13447

thanks,
Pádraig.


-- 
To UNSUBSCRIBE, email to debian-bugs-dist-requ...@lists.debian.org
with a subject of unsubscribe. Trouble? Contact listmas...@lists.debian.org

Bug#739752: bug#17010: Bug#739752: coreutils: ln segfaults when run with --relative and an empty target

2014-03-13 Thread Jim Meyering 
On Thu, Mar 13, 2014 at 7:22 PM, Pádraig Brady p...@draigbrady.com wrote:
 On 03/14/2014 01:42 AM, Jim Meyering wrote:
 From a6d2db8b6dfe15344aba4aefe9545eb3a4876d45 Mon Sep 17 00:00:00 2001
 From: Jim Meyering meyer...@fb.com
 Date: Thu, 13 Mar 2014 17:05:04 -0700
 Subject: [PATCH] ln: with -sr, don't segfault for a TARGET of ''

 Prior to this change, ln -sr '' F would segfault, attempting
 to read path2[1] in relpath.c's path_common_prefix function.
 This problem arises whenever canonicalize_filename_mode returns
 NULL.
 * src/ln.c (convert_abs_rel): Call relpath only when
 both canonicalize_filename_mode calls return non-NULL.
 * tests/ln/relative.sh: Add a test to trigger this failure.
 * THANKS.in: List reporter's name/address.
 * NEWS (Bug fixes): Mention it.
 Reported by Erik Bernstein in 739...@bugs.debian.org.

 We can amend with the now allocated:

   Fixes http://bugs.gnu.org/17010

Done.

 diff --git a/NEWS b/NEWS
...
 +  ln -sr '' F no longer segfaults: now it fails with the expected diagnostic

 Probably should add:

  [bug introduced with the --relative feature in coreutils-8.16]

Definitely.  Thanks.

 diff --git a/src/ln.c b/src/ln.c
 index aab9cf2..6726699 100644
 --- a/src/ln.c
 +++ b/src/ln.c
 @@ -149,13 +149,17 @@ convert_abs_rel (const char *from, const char *target)
char *realdest = canonicalize_filename_mode (targetdir, CAN_MISSING);
char *realfrom = canonicalize_filename_mode (from, CAN_MISSING);

 Interesting. So canonicalize_filename_mode() can fail in this case,
 even with CAN_MISSING. It's unexpected that c_f_m() sets errno=ENOENT
 when CAN_MISSING is set. I wonder should we change that instead
 in gnulib? With CAN_MISSING I would expect this function to work
 on arbitrary strings, including the empty string.

What would you have c_f_m(, CAN_MISSING) return?
I know of no absolute name corresponding to the dot-relative empty string.

 diff --git a/tests/ln/relative.sh b/tests/ln/relative.sh
 +# Expect this to fail with exit status 1.
 +# Prior to coreutils-8.23, it would segfault.
 +ln -sr '' F
 +test $? = 1 || fail=1

 Won't the ln succeed on FreeBSD as per:
 http://bugs.gnu.org/13447

You're right.  Good catch.  Adjusted as well.
Thanks for the speedy and thorough review.
Here's a revised patch:
From 0093ac8d57a0f1a16fd09d98f6a524dddb6053e7 Mon Sep 17 00:00:00 2001
From: Jim Meyering meyer...@fb.com
Date: Thu, 13 Mar 2014 17:05:04 -0700
Subject: [PATCH] ln: with -sr, don't segfault for a TARGET of ''

Prior to this change, ln -sr '' F would segfault, attempting
to read path2[1] in relpath.c's path_common_prefix function.
This problem arises whenever canonicalize_filename_mode returns
NULL.
* src/ln.c (convert_abs_rel): Call relpath only when
both canonicalize_filename_mode calls return non-NULL.
* tests/ln/relative.sh: Add a test to trigger this failure.
* THANKS.in: List reporter's name/address.
* NEWS (Bug fixes): Mention it.
Reported by Erik Bernstein in 739...@bugs.debian.org.
Fixes http://bugs.gnu.org/17010.
---
 NEWS |  3 +++
 THANKS.in|  1 +
 src/ln.c | 16 ++--
 tests/ln/relative.sh |  5 +
 4 files changed, 19 insertions(+), 6 deletions(-)

diff --git a/NEWS b/NEWS
index 62966b2..35d48e5 100644
--- a/NEWS
+++ b/NEWS
@@ -25,6 +25,9 @@ GNU coreutils NEWS-*- 
outline -*-
   it would display an error, requiring --no-dereference to avoid the issue.
   [bug introduced in coreutils-5.3.0]

+  ln -sr '' F no longer segfaults.  Now works as expected.
+  [bug introduced with the --relative feature in coreutils-8.16]
+
   shuf --repeat no longer dumps core if the input is empty.
   [bug introduced with the --repeat feature in coreutils-8.22]

diff --git a/THANKS.in b/THANKS.in
index 561d18c..2670265 100644
--- a/THANKS.in
+++ b/THANKS.in
@@ -193,6 +193,7 @@ Eric G. Miller  e...@jps.net
 Eric Pementepeme...@northpark.edu
 Eric S. Raymond e...@snark.thyrsus.com
 Erik Bennettbenn...@cvo.oneworld.com
+Erik Bernstein  e...@fscking.org
 Erik Corry  e...@kroete2.freinet.de
 Felix Lee   f...@teleport.com
 Felix Rauch Valenti fra...@cse.unsw.edu.au
diff --git a/src/ln.c b/src/ln.c
index aab9cf2..6726699 100644
--- a/src/ln.c
+++ b/src/ln.c
@@ -149,13 +149,17 @@ convert_abs_rel (const char *from, const char *target)
   char *realdest = canonicalize_filename_mode (targetdir, CAN_MISSING);
   char *realfrom = canonicalize_filename_mode (from, CAN_MISSING);

-  /* Write to a PATH_MAX buffer.  */
-  char *relative_from = xmalloc (PATH_MAX);
-
-  if (!relpath (realfrom, realdest, relative_from, PATH_MAX))
+  char *relative_from = NULL;
+  if (realdest  realfrom)
 {
-  free (relative_from);
-  relative_from = NULL;
+  /* Write to a PATH_MAX buffer.  */
+  relative_from = xmalloc (PATH_MAX);
+
+  if

Bug#739752: coreutils: ln segfaults when run with --relative and an empty target

2014-02-22 Thread Erik Bernstein 
Package: coreutils
Version: 8.21-1
Severity: normal

Hi,

when ln is run with --relative --symbolic and and empty string as the
target, it ungracefully dies with a segmentation fault. The memory
violation appears to happen in src/relpath.c:38 when the two input paths
are checked for leading slashes:

  if ((path1[1] == '/') != (path2[1] == '/'))

How to reproduce:
[1] Open a terminal
[2] run: ln -sr '' foobar

Result: segmentation fault  ln -sr '' foobar
Expected result: Some kind of error message

Trace:

#0  0x00403370 in path_common_prefix (path1=0x611030 /root, 
path2=0x0) at src/relpath.c:38
#1  0x004034fe in relpath (can_fname=0x0, can_reldir=0x611030 /root, 
buf=0x611050 , len=4096) at src/relpath.c:93
#2  0x00402003 in convert_abs_rel (from=0x7fffe3df , 
target=0x7fffe3e0 foobar) at src/ln.c:144
#3  0x004025ca in do_link (source=0x7fffe3df , 
dest=0x7fffe3e0 foobar) at src/ln.c:284
#4  0x0040331c in main (argc=4, argv=0x7fffe108) at src/ln.c:631

cheers,

  erik


-- System Information:
Debian Release: jessie/sid
  APT prefers testing
  APT policy: (500, 'testing')
Architecture: amd64 (x86_64)
Foreign Architectures: i386

Kernel: Linux 3.12-1-amd64 (SMP w/4 CPU cores)
Locale: LANG=en_US.utf8, LC_CTYPE=en_US.utf8 (charmap=UTF-8)
Shell: /bin/sh linked to /bin/dash

Versions of packages coreutils depends on:
ii  libacl1  2.2.52-1
ii  libattr1 1:2.4.47-1
ii  libc62.17-97
ii  libselinux1  2.2.2-1

coreutils recommends no packages.

coreutils suggests no packages.

-- no debconf information


-- 
To UNSUBSCRIBE, email to debian-bugs-dist-requ...@lists.debian.org
with a subject of unsubscribe. Trouble? Contact listmas...@lists.debian.org