bug#63931: ls colors one symlink too much as non-broken in symlink chain

2023-06-06 Thread Paul Eggert

On 6/6/23 13:32, Pádraig Brady wrote:

I'll think a bit more about it and add a test before applying.


I wouldn't test that the 'ls' output exactly matches the 'cat' output. 
For example, it should be OK for stat_for_mode to pass the parent 
directory fd (instead of AT_FDCWD) to statx, for efficiency, and in that 
case the number of symbolic links traversed by 'ls' won't match the 
number traversed by 'cat', because the full name might traverse symlinks 
that statx (dirfd, ...) won't.


And it's OK if these two numbers don't match, as users shouldn't assume 
that the ELOOP limit is a constant.


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).






bug#63931: ls colors one symlink too much as non-broken in symlink chain

2023-06-06 Thread Pádraig Brady

On 06/06/2023 18:24, Martin Schulte wrote:

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,


This is because we lstat() the link, and stat() the target,
I suppose as a small optimization in the normal case.
The following adjusts to stat the link instead,
which works for your case and passes existing tests.

I'll think a bit more about it and add a test before applying.

cheers,
Pádraig

diff --git a/src/ls.c b/src/ls.c
index 71d94fd6a..77be54248 100644
--- a/src/ls.c
+++ b/src/ls.c
@@ -3586,7 +3586,7 @@ gobble_file (char const *name, enum filetype type, ino_t 
inode,
  they won't be traced and when no indicator is needed.  */
   if (linkname
   && (file_type <= indicator_style || check_symlink_mode)
-  && stat_for_mode (linkname, &linkstats) == 0)
+  && stat_for_mode (full_name, &linkstats) == 0)
 {
   f->linkok = true;
   f->linkmode = linkstats.st_mode;






bug#63931: ls colors one symlink too much as non-broken in symlink chain

2023-06-06 Thread Martin Schulte
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





bug#63850: cp fails for files > 2 GB if copy offload is unsupported

2023-06-06 Thread Pádraig Brady

On 06/06/2023 11:02, Sam James wrote:


Paul Eggert  writes:


On 2023-06-05 22:26, Sam James wrote:

It's just that linux-headers is
a special case


Indeed it is. And apparently glibc avoids the copy_file_range bugs by
never, ever using copy_file_range internally - which explains why
glibc hasn't run into this backward compatibility issue. Presumably
once glibc starts assuming kernel 5.3 or later, it can start using
copy_file_range internally.

Anyway, thanks for explaining. I installed the patch I mentioned into
Gnulib and have updated coreutils accordingly on Savannah
master. Please give it a try.


Thanks Paul. Do you know if there's any other cases of this in gnulib?

(or whether, by chance, if you know off the top of your head if any
other gnulib consumers definitely use copy_file_range?)


Both google and cs.github.com suggest only coreutils uses copy-file-range from 
gnulib:
https://www.google.com/search?q=filetype%3Aconf+%22copy-file-range%22

cheers,
Pádraig





bug#63850: cp fails for files > 2 GB if copy offload is unsupported

2023-06-06 Thread Sam James

Paul Eggert  writes:

> On 2023-06-05 22:26, Sam James wrote:
>> It's just that linux-headers is
>> a special case
>
> Indeed it is. And apparently glibc avoids the copy_file_range bugs by
> never, ever using copy_file_range internally - which explains why
> glibc hasn't run into this backward compatibility issue. Presumably
> once glibc starts assuming kernel 5.3 or later, it can start using
> copy_file_range internally.
>
> Anyway, thanks for explaining. I installed the patch I mentioned into
> Gnulib and have updated coreutils accordingly on Savannah
> master. Please give it a try.

Thanks Paul. Do you know if there's any other cases of this in gnulib?

(or whether, by chance, if you know off the top of your head if any
other gnulib consumers definitely use copy_file_range?)


signature.asc
Description: PGP signature


bug#63850: cp fails for files > 2 GB if copy offload is unsupported

2023-06-06 Thread Pádraig Brady

On 06/06/2023 06:48, Paul Eggert wrote:

On 2023-06-05 22:26, Sam James wrote:

It's just that linux-headers is
a special case


Indeed it is. And apparently glibc avoids the copy_file_range bugs by
never, ever using copy_file_range internally - which explains why glibc
hasn't run into this backward compatibility issue. Presumably once glibc
starts assuming kernel 5.3 or later, it can start using copy_file_range
internally.


Yes, glibc has the --enable-kernel configure option
to allow setting the compatibility range independently
from the build host (kernel headers).
In my experience a distro needs to support quite a range of kernel versions
mainly due to certain hardware platforms being tied to older kernels.


Anyway, thanks for explaining. I installed the patch I mentioned into
Gnulib and have updated coreutils accordingly on Savannah master. Please
give it a try.


Thanks for making the change Paul.

Pádraig