GNU bug report logs -
#17010
Bug#739752: coreutils: ln segfaults when run with --relative and an empty target
Previous Next
Reported by: Jim Meyering <jim <at> meyering.net>
Date: Fri, 14 Mar 2014 01:44:02 UTC
Severity: normal
Done: Jim Meyering <jim <at> meyering.net>
Bug is archived. No further changes may be made.
To add a comment to this bug, you must first unarchive it, by sending
a message to control AT debbugs.gnu.org, with unarchive 17010 in the body.
You can then email your comments to 17010 AT debbugs.gnu.org in the normal way.
Toggle the display of automated, internal messages from the tracker.
Report forwarded
to
bug-coreutils <at> gnu.org:
bug#17010; Package
coreutils.
(Fri, 14 Mar 2014 01:44:02 GMT)
Full text and
rfc822 format available.
Acknowledgement sent
to
Jim Meyering <jim <at> meyering.net>:
New bug report received and forwarded. Copy sent to
bug-coreutils <at> gnu.org.
(Fri, 14 Mar 2014 01:44:02 GMT)
Full text and
rfc822 format available.
Message #5 received at submit <at> debbugs.gnu.org (full text, mbox):
[Message part 1 (text/plain, inline)]
On Sat, Feb 22, 2014 at 1:57 AM, Erik Bernstein <erik <at> 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:
[k.txt (text/plain, attachment)]
Information forwarded
to
bug-coreutils <at> gnu.org:
bug#17010; Package
coreutils.
(Fri, 14 Mar 2014 02:23:02 GMT)
Full text and
rfc822 format available.
Message #8 received at 17010 <at> debbugs.gnu.org (full text, mbox):
On 03/14/2014 01:42 AM, Jim Meyering wrote:
> From a6d2db8b6dfe15344aba4aefe9545eb3a4876d45 Mon Sep 17 00:00:00 2001
> From: Jim Meyering <meyering <at> 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 739752 <at> 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.
Information forwarded
to
bug-coreutils <at> gnu.org:
bug#17010; Package
coreutils.
(Fri, 14 Mar 2014 03:46:01 GMT)
Full text and
rfc822 format available.
Message #11 received at 17010 <at> debbugs.gnu.org (full text, mbox):
[Message part 1 (text/plain, inline)]
On Thu, Mar 13, 2014 at 7:22 PM, Pádraig Brady <P <at> 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 <meyering <at> 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 739752 <at> 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:
[k.txt (text/plain, attachment)]
Information forwarded
to
bug-coreutils <at> gnu.org:
bug#17010; Package
coreutils.
(Fri, 14 Mar 2014 11:51:01 GMT)
Full text and
rfc822 format available.
Message #14 received at 17010 <at> debbugs.gnu.org (full text, mbox):
On 03/14/2014 03:44 AM, Jim Meyering wrote:
> On Thu, Mar 13, 2014 at 7:22 PM, Pádraig Brady <P <at> 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.
Reply sent
to
Jim Meyering <jim <at> meyering.net>:
You have taken responsibility.
(Fri, 14 Mar 2014 16:28:02 GMT)
Full text and
rfc822 format available.
Notification sent
to
Jim Meyering <jim <at> meyering.net>:
bug acknowledged by developer.
(Fri, 14 Mar 2014 16:28:03 GMT)
Full text and
rfc822 format available.
Message #19 received at 17010-done <at> debbugs.gnu.org (full text, mbox):
On Fri, Mar 14, 2014 at 4:49 AM, Pádraig Brady <P <at> draigbrady.com> wrote:
...
> Hence since c_f_m() can validly fail even with CAN_MISSING,
> I agree your patch is correct.
>
> Please push.
Done.
Information forwarded
to
bug-coreutils <at> gnu.org:
bug#17010; Package
coreutils.
(Sat, 15 Mar 2014 04:14:01 GMT)
Full text and
rfc822 format available.
Message #22 received at 17010-done <at> debbugs.gnu.org (full text, mbox):
[Message part 1 (text/plain, inline)]
On Fri, Mar 14, 2014 at 9:27 AM, Jim Meyering <jim <at> meyering.net> wrote:
> On Fri, Mar 14, 2014 at 4:49 AM, Pádraig Brady <P <at> 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:
[k.txt (text/plain, attachment)]
bug archived.
Request was from
Debbugs Internal Request <help-debbugs <at> gnu.org>
to
internal_control <at> debbugs.gnu.org.
(Sat, 12 Apr 2014 11:24:04 GMT)
Full text and
rfc822 format available.
This bug report was last modified 10 years and 27 days ago.
Previous Next
GNU bug tracking system
Copyright (C) 1999 Darren O. Benham,
1997,2003 nCipher Corporation Ltd,
1994-97 Ian Jackson.