GNU bug report logs - #63931
ls colors one symlink too much as non-broken in symlink chain

Previous Next

Full log

View this message in rfc822 format

From: help-debbugs <at> gnu.org (GNU bug Tracking System)
To: Pádraig Brady <P <at> draigBrady.com>
Cc: tracker <at> debbugs.gnu.org
Subject: bug#63931: closed (ls colors one symlink too much as non-broken
 in symlink chain)
Date: Wed, 07 Jun 2023 15:02:02 +0000

[Message part 1 (text/plain, inline)]

Your message dated Wed, 7 Jun 2023 16:01:42 +0100
with message-id <7a38b7fc-bf5e-2f42-2a77-9eb3d71dec79 <at> draigBrady.com>
and subject line Re: bug#63931: ls colors one symlink too much as non-broken in symlink chain
has caused the debbugs.gnu.org bug report #63931,
regarding ls colors one symlink too much as non-broken in symlink chain
to be marked as done.

(If you believe you have received this mail in error, please contact
help-debbugs <at> gnu.org.)


-- 
63931: https://debbugs.gnu.org/cgi/bugreport.cgi?bug=63931
GNU Bug Tracking System
Contact help-debbugs <at> gnu.org with problems

[Message part 2 (message/rfc822, inline)]

From: Martin Schulte <gnu <at> schrader-schulte.de>
To: bug-coreutils <at> gnu.org
Subject: ls colors one symlink too much as non-broken in symlink chain
Date: Tue, 6 Jun 2023 19:24:26 +0200

Hello coreutils-maintainers,

I create a long chain of symlinks - ls colors the 41st element as ok while the kernel already gives up after 40 symlinks:

$ uname -a
Linux martnix4 5.10.0-23-amd64 #1 SMP Debian 5.10.179-1 (2023-05-12) x86_64 GNU/Linux
$ ls --version | head -n 1
ls (GNU coreutils) 8.32
$ mkdir empty
$ cd empty
$ last=00 ; touch $last ; for ln in {01..50}; do ln -s $last $ln; last=$ln; done
$ ls --color -l {38..42}
lrwxrwxrwx 1 u g 2 Jun  6 19:00 38 -> 37   <- 38 colored as symlink
lrwxrwxrwx 1 u g 2 Jun  6 19:00 39 -> 38   <- 39 colored as symlink
lrwxrwxrwx 1 u g 2 Jun  6 19:00 40 -> 39   <- 40 colored as symlink
lrwxrwxrwx 1 u g 2 Jun  6 19:00 41 -> 40   <- 41 colored as symlink
lrwxrwxrwx 1 u g 2 Jun  6 19:00 42 -> 41   <- 42 and 41 colored as broken link
$ cat 40
$ cat 41
cat: 41: Too many levels of symbolic links

This could be reproduced with "ls (GNU coreutils) 9.3.45-6a618" compiled from the git repository.

Best regards,

Martin

[Message part 3 (message/rfc822, inline)]

From: Pádraig Brady <P <at> draigBrady.com>
To: Martin Schulte <gnu <at> schrader-schulte.de>
Cc: 63931-done <at> debbugs.gnu.org
Subject: Re: bug#63931: ls colors one symlink too much as non-broken in
 symlink chain
Date: Wed, 7 Jun 2023 16:01:42 +0100

[Message part 4 (text/plain, inline)]

On 07/06/2023 13:23, Pádraig Brady wrote:
> On 07/06/2023 10:56, Martin Schulte wrote:
>> Hello Paul, hello Pádraig,
>>
>> thanks a lot for your analysis and explanations!
>>
>>> With that in mind, the code change you proposed is reasonably innocuous,
>>> although it slows things down a bit in the usual case. Not sure it's
>>> worth doing (I guess it does fix a race but there are other unfixable
>>> races in this area....).
>>
>> I first supposed that the effect was caused by an off-by-one problem, but after understanding it, I think you are right here to ask if changing the behaviour is worth doing.
> 
> Well changing it makes things more consistent,
> and also simplifies ls.c as we can remove the make_link_name() function.
> 
>> Looking at
>>
>> $ ls {38..42}
>> ls: cannot access '41': Too many levels of symbolic links
>> ls: cannot access '42': Too many levels of symbolic links
>> 38  39  40
>>
>> shows that there would be more issues to consider in this case.
> 
> Indeed. That can be addressed with the following.
> 
> cheers,
> Pádraig
> 
> diff --git a/src/ls.c b/src/ls.c
> index fbeb9b6dc..33f692bb4 100644
> --- a/src/ls.c
> +++ b/src/ls.c
> @@ -3480,7 +3480,7 @@ gobble_file (char const *name, enum filetype type, ino_t inode,
>                    break;
> 
>                  need_lstat = (err < 0
> -                            ? errno == ENOENT
> +                            ? (errno == ENOENT || errno == ELOOP)
>                                : ! S_ISDIR (f->stat.st_mode));
>                  if (!need_lstat)
>                    break;

The complete two patches in this area are attached,
which I'll apply a bit later.

Marking this as done,

thanks,
Pádraig

[0001-ls-use-more-standard-symlink-traversal.patch (text/x-patch, attachment)]

[0002-ls-display-command-line-symlinks-that-return-ELOOP.patch (text/x-patch, attachment)]

Previous Next

GNU bug tracking system
Copyright (C) 1999 Darren O. Benham, 1997,2003 nCipher Corporation Ltd, 1994-97 Ian Jackson.