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

[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 <address@hidden>
> 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 address@hidden

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.