bug#18621: [BUG] wc -c incorrectly counts bytes in /sys
Paul Eggert wrote:
How about if we change usable_st_size to return false for these proc files
Attached is a better idea, I hope. I audited the coreutils code to look for
problematic uses of SEEK_END or st_size when reading files (I didn't look at
writing; one can of worms at a time).
The attached patch still needs a changelog entry and test cases. The basic idea
is to not trust st_size when it's = ST_BLKSIZE. This fixes bugs in 'head',
'od', 'split', 'tac', 'tail', and 'wc' when applied to input files in proc or
sysfs file systems.
Here's an example bug fixed by this patch:
$ cat /sys/kernel/profiling
0
$ tail -2c /sys/kernel/profiling
$
This should be:
$ cat /sys/kernel/profiling
0
$ tail -2c /sys/kernel/profiling
0
$
diff --git a/src/head.c b/src/head.c
index d2f1fce..f6f890a 100644
--- a/src/head.c
+++ b/src/head.c
@@ -36,6 +36,7 @@
#include quote.h
#include quotearg.h
#include safe-read.h
+#include stat-size.h
#include xfreopen.h
#include xstrtol.h
@@ -206,13 +207,42 @@ copy_fd (int src_fd, uintmax_t n_bytes)
return COPY_FD_OK;
}
-/* Print all but the last N_ELIDE bytes from the input available via
- the non-seekable file descriptor FD. Return true upon success.
+/* Call lseek (FD, OFFSET, WHENCE), where file descriptor FD
+ corresponds to the file FILENAME. WHENCE must be SEEK_SET or
+ SEEK_CUR. Return the resulting offset. Give a diagnostic and
+ return -1 if lseek fails. */
+
+static off_t
+elseek (int fd, off_t offset, int whence, char const *filename)
+{
+ off_t new_offset = lseek (fd, offset, whence);
+ char buf[INT_BUFSIZE_BOUND (offset)];
+
+ if (new_offset 0)
+error (0, errno,
+ _(whence == SEEK_SET
+ ? N_(%s: cannot seek to offset %s)
+ : N_(%s: cannot seek to relative offset %s)),
+ quotearg_colon (filename),
+ offtostr (offset, buf));
+
+ return new_offset;
+}
+
+/* For an input file with name FILENAME and descriptor FD,
+ output all but the last N_ELIDE_0 bytes.
+ If CURRENT_POS is nonnegative, assume that the input file is
+ positioned at CURRENT_POS and that it should be repositioned to
+ just before the elided bytes before returning.
+ Return true upon success.
Give a diagnostic and return false upon error. */
static bool
-elide_tail_bytes_pipe (const char *filename, int fd, uintmax_t n_elide_0)
+elide_tail_bytes_pipe (const char *filename, int fd, uintmax_t n_elide_0,
+ off_t current_pos)
{
size_t n_elide = n_elide_0;
+ uintmax_t desired_pos = current_pos;
+ bool ok = true;
#ifndef HEAD_TAIL_PIPE_READ_BUFSIZE
# define HEAD_TAIL_PIPE_READ_BUFSIZE BUFSIZ
@@ -251,7 +281,6 @@ elide_tail_bytes_pipe (const char *filename, int fd,
uintmax_t n_elide_0)
if (n_elide = HEAD_TAIL_PIPE_BYTECOUNT_THRESHOLD)
{
- bool ok = true;
bool first = true;
bool eof = false;
size_t n_to_read = READ_BUFSIZE + n_elide;
@@ -293,22 +322,26 @@ elide_tail_bytes_pipe (const char *filename, int fd,
uintmax_t n_elide_0)
/* Output any (but maybe just part of the) elided data from
the previous round. */
if (! first)
-xwrite_stdout (b[!i] + READ_BUFSIZE, n_elide - delta);
+{
+ desired_pos += n_elide - delta;
+ xwrite_stdout (b[!i] + READ_BUFSIZE, n_elide - delta);
+}
first = false;
if (n_elide n_read)
-xwrite_stdout (b[i], n_read - n_elide);
+{
+ desired_pos += n_read - n_elide;
+ xwrite_stdout (b[i], n_read - n_elide);
+}
}
free (b[0]);
- return ok;
}
else
{
/* Read blocks of size READ_BUFSIZE, until we've read at least n_elide
bytes. Then, for each new buffer we read, also write an old one. */
- bool ok = true;
bool eof = false;
size_t n_read;
bool buffered_enough;
@@ -357,7 +390,10 @@ elide_tail_bytes_pipe (const char *filename, int fd,
uintmax_t n_elide_0)
buffered_enough = true;
if (buffered_enough)
-xwrite_stdout (b[i_next], n_read);
+{
+ desired_pos += n_read;
+ xwrite_stdout (b[i_next], n_read);
+}
}
/* Output any remainder: rem bytes from b[i] + n_read. */
@@ -366,6 +402,7 @@ elide_tail_bytes_pipe (const char *filename, int fd,
uintmax_t n_elide_0)
if (buffered_enough)
{
size_t n_bytes_left_in_b_i = READ_BUFSIZE - n_read;
+ desired_pos += rem;
if (rem n_bytes_left_in_b_i)
{
xwrite_stdout (b[i] + n_read, rem);
@@ -392,6 +429,7 @@ elide_tail_bytes_pipe (const char *filename, int fd,
uintmax_t n_elide_0)
*/
size_t y = READ_BUFSIZE - rem;
size_t x = n_read - y;
+ desired_pos += x;
bug#8527: cp/mv in coreutils don't respect the default ACL of parent
f0r...@free.fr wrote: I can confirm. Tests show that the parent folder ACL Default mask is not inherited as the ACL Access mask of the file|dir created by cp|mv. What file system and core utils are you using? Are you using a file system that has alternate user-data forks or extended attributes that have them included by default? Or are you using a file system where they were added on as a super-user control'd option? Have you tried copying them as root? The reason I ask, is that I just tried it and it appears to work: 1) First the dir: cd /tmp llg -d /tmp drwxrwxrwt 25 root root 8192 Oct 7 02:21 /tmp/ lsacl /tmp [u::rwx,g::rwx,o::rwx] /tmp #default ACL from mode bits 2) Create file with 'touch' touch x # new file Ishtar:/tmp llg x -rw-rw-r-- 1 law lawgroup 0 Oct 7 02:26 x lsacl [u::rw-,g::rw-,o::r--] x #default ACL 3) now I'll copy in a *directory* that has both types of ACL's on it, but not specifying that any permissions be copied: ll -d /Media/Library/_artwork/test #source drwxrwsr-x+ 2 10 Oct 7 02:33 /Media/Library/_artwork/test/ Ishtar:/tmp lsacl /Media/Library/_artwork/test [u::rwx,u:Media:rwx,g::rwx,g:Media:rwx,m::rwx,o::r-x/u::rwx,u:Media:rwx,g::rwx,g:Media:rwx,m::rwx,o::r-x] /Media/Library/_artwork/test (note, 2nd acl is default dir (lsacl uses chacl -l) Ishtar:/tmp 'cp' -r /Media/Library/_artwork/test . #recursive to tmp Ishtar:/tmp llg -d test drwxrwxr-x 2 law lawgroup 6 Oct 7 02:34 test/ Ishtar:/tmp lsacl test #no attr indicated [u::rwx,g::rwx,o::r-x] test #default ACL shown So far all seems fine. 4) Now lets copy the perms too: Ishtar:/tmp rd test Ishtar:/tmp 'cp' -a /Media/Library/_artwork/test . Ishtar:/tmp llg -d test drwxrwsr-x+ 2 law Media 6 Oct 7 02:33 test/ Ishtar:/tmp lsacl test #same ACL as source [u::rwx,u:Media:rwx,g::rwx,g:Media:rwx,m::rwx,o::r-x/u::rwx,u:Media:rwx,g::rwx,g:Media:rwx,m::rwx,o::r-x] test 5) create file in that dir: Ishtar:/tmp cd test Ishtar:/tmp/test touch touched_file Ishtar:/tmp/test llg touched_file -rw-rw-r--+ 1 law Media 0 Oct 7 02:42 touched_file Ishtar:/tmp/test lsacl touched_file [u::rw-,u:Media:rwx,g::rwx,g:Media:rwx,m::rw-,o::r--] touched_file --- File has expected inherited ACL. 6) Now ... lets use cp to copy a file w/o acls in: (first create normal file under /tmp): echo perm test/tmp/perm.txt Ishtar:/tmp/test llg /tmp/perm.txt -rw-rw-r-- 1 law lawgroup 10 Oct 7 02:59 /tmp/perm.txt Ishtar:/tmp/test lsacl /tmp/perm.txt [u::rw-,g::rw-,o::r--] /tmp/perm.txt 'cp' /tmp/perm.txt . Ishtar:/tmp/test llg perm.txt -rw-rw-r--+ 1 law Media 10 Oct 7 03:01 perm.txt Ishtar:/tmp/test lsacl perm.txt [u::rw-,u:Media:rwx,g::rwx,g:Media:rwx,m::rw-,o::r--] perm.txt 8) Looks the same to me...However, check this out: Ishtar:/tmp/test rm perm.txt Ishtar:/tmp/test cp /tmp/perm.txt . Ishtar:/tmp/test llg /tmp/perm.txt -rw-rw-r-- 1 law lawgroup 10 Oct 7 02:59 /tmp/perm.txt Ishtar:/tmp/test lsacl perm.txt No acl this time, but same copy...or was it? Note I was careful to use 'cp' most of the time when copying except this last time, cuz: alias cp alias cp='cp --preserve=mode,timestamps' my normal cp is an alias -- that says to preserve the mode. It wouldn't be able to do that if it allowed the default ACL to be set on the file. -- So, I don't know if this is related to your problem, but cp appears to be working correctly here filesystem = xfs (acls are always on as they came with the filesystem). kernel= Linux Ishtar 3.16.2-Isht-Van #1 SMP PREEMPT Tue Sep 9 18:26:43 PDT 2014 x86_64 x86_64 x86_64 GNU/Linux == If this was any help -- great, if it was an annoyance, just delete it and I can claim my dog ate my keyboard... (funny things come out of dogs stomachs ;-))...
bug#8527: cp/mv in coreutils don't respect the default ACL of parent
Thank you Linda for extensive answer. Just an additional info before I reply your questions: for my own tests I didn't use /tmp as target because the sticky bit could do something special (not sure). Instead I used /srv/test that I chown me:writers , set chmod -R u:rwX,g:srwX then setfacl --set as needed all this as root. The goal being having a group writers rwX, another group readers with rX on the tree and o:---, and ignore source perms if any. What file system and core utils are you using? My target file system is ext4 (default mount options include acl and user_xattr , coreutils is 8.21 kernel is 3.13.0-36-generic #63-Ubuntu SMP Wed Sep 3 21:30:07 UTC 2014 x86_64 GNU/Linux with embedded acl support out of the box). Are you using a file system that has alternate user-data forks or extended attributes that have them included by default? Or are you using a file system where they were added on as a super-user control'd option? Have you tried copying them as root? I know this: from local, umask=0002 from ssh, umask=0022 no cp aliases, I just need/use the default, i.e. do-not-preserve-perms All my tests below are run locally. So I wrote a script that echoes each line: sudo ~/acl.sh 0 mkdir -pv /srv/test 0 setfacl -bk /srv/test 0 rm -rf /srv/test/* ownership of /srv/test was kept as me:writers 0 chown -Rv me:writers /srv/test mode of /srv/test/ was changed from 2770 (rwxrws---) to (-) 0 (removed all bits) mode of /srv/test/ was changed from (-) to 2770 (rwxrws---) 0 chmod -Rv u+rwX,g+srwX /srv/test 0 setfacl -R --set d:u::rwx,d:g::rwx,d:g:writers:rwx,d:u:reader:rx,d:g:reader:rx,d:o::---,d:m::rwx /srv/test getfacl: remove first / out of absolute path names # file: srv/test USER me rwx rwx user readerr-x GROUP writers rwx rwx group readerr-x group writers rwx mask rwx other --- --- 0 setfacl -R --set u::rwX,g::rwX,u:reader:rX,g:writers:rwX,g:reader:rx,o::---,m::rwX /srv/test getfacl: remove first / out of absolute path names # file: srv/test USER me rwx rwx user reader r-x r-x GROUP writers rwx rwx group reader r-x r-x group writers rwx rwx maskrwx rwx other --- --- So at the moment this last command shows all is alright Now, let's copy me@pc:/srv$ cp -r /media/me/USPEED/200402/ /srv/test me@pc:/srv$ getfacl -t /srv/test/200402/ getfacl: remove first / out of absolute path names # file: srv/test/200402/ USER me rwx rwx user reader R-X r-x GROUP writers RWX rwx group reader R-X r-x group writers RWX rwx mask--- rwx other --- --- ***problems begin: defaults ACL are kept OK (right perm column, *** ***but Access ACL are lost (capitalized in left column by -t are the denied perms because mask is lost, do not confuse with cap X in chmod)*** ***only file owner can traverse, nobody else can)*** me@pc:/srv$ getfacl -t /srv/test/200402/P2220368.JPG getfacl: remove first / out of absolute path names # file: srv/test/200402/P2220368.JPG USER me rw- user reader r-X GROUP writers rWX group reader r-X group writers rWX maskr-- other --- *** Here one see writers lost the write perm, and reader could read if only he could traverse above*** Do the same by creation: me@pc:/srv$ mkdir test/handdir me@pc:/srv$ touch test/handdir/file me@pc:/srv$ getfacl -Rt test/handdir/ # file: test/handdir/ USER me rwx rwx user reader r-x r-x GROUP writers rwx rwx group reader r-x r-x group writers rwx rwx maskrwx rwx other --- --- # file: test/handdir//file USER me rw- user reader r-X GROUP writers rwX group reader r-X group writers rwX maskrw- other --- ***all is OK this way*** The reason I ask, is that I just tried it and it appears to work: 1) First the dir: cd /tmp llg -d /tmp drwxrwxrwt 25 root root 8192 Oct 7 02:21 /tmp/ lsacl /tmp [u::rwx,g::rwx,o::rwx] /tmp #default ACL from mode bits 2) Create file with 'touch' touch x # new file Ishtar:/tmp llg x -rw-rw-r-- 1 law lawgroup 0 Oct 7 02:26 x lsacl [u::rw-,g::rw-,o::r--] x #default ACL 3) now I'll copy in a *directory* that has both types of ACL's on it, but not specifying that any permissions be copied: ll -d /Media/Library/_artwork/test #source drwxrwsr-x+ 2 10 Oct 7 02:33
bug#18621: [BUG] wc -c incorrectly counts bytes in /sys
Paul Eggert wrote:
The attached patch still needs a changelog entry and test cases.
I wrote those up and pushed the attached patch; this should fix the bug so I'm
closing the bug report.
From c3c87a92ba016495c20d13b80a42d750f3e0fba0 Mon Sep 17 00:00:00 2001
From: Paul Eggert egg...@cs.ucla.edu
Date: Tue, 7 Oct 2014 16:46:08 -0700
Subject: [PATCH] wc: don't miscount /sys and similar file systems
Fix similar problems in head, od, split, tac, and tail.
Reported by George Shuklin in: http://bugs.gnu.org/18621
* NEWS: Document this.
* src/head.c (elseek): Move up.
(elide_tail_bytes_pipe, elide_tail_lines_pipe): New arg
CURRENT_POS. All uses changed.
(elide_tail_bytes_file, elide_tail_lines_file):
New arg ST and remove arg SIZE. All uses changed.
* src/head.c (elide_tail_bytes_file):
* src/od.c (skip): Avoid optimization for /sys files, where
st_size is bogus and st_size == st_blksize.
Don't report error at EOF when not optimizing.
* src/head.c, src/od.c, src/tail.c: Include stat-size.h.
* src/split.c (input_file_size): New function.
(bytes_split, lines_chunk_split, bytes_chunk_extract): New arg
INITIAL_READ. All uses changed. Use it to double-check st_size.
* src/tac.c (tac_seekable): New arg FILE_POS. All uses changed.
(copy_to_temp): Return size of temp file. All uses changed.
* src/tac.c (tac_seekable):
* src/tail.c (tail_bytes):
* src/wc.c (wc):
Don't trust st_size; double-check by reading.
* src/wc.c (wc): New arg CURRENT_POS. All uses changed.
* tests/local.mk (all_tests): Add tests/misc/wc-proc.sh,
tests/misc/od-j.sh, tests/tail-2/tail-c.sh.
* tests/misc/head-c.sh:
* tests/misc/tac-2-nonseekable.sh:
* tests/split/b-chunk.sh:
Add tests for problems with /proc and /sys files.
* tests/misc/od-j.sh, tests/misc/wc-proc.sh, tests/tail-2/tail-c.sh:
New files.
---
NEWS| 3 +
src/head.c | 147
src/od.c| 23 +--
src/split.c | 146 ++-
src/tac.c | 70 ---
src/tail.c | 43 +---
src/wc.c| 45 ++--
tests/local.mk | 3 +
tests/misc/head-c.sh| 12
tests/misc/od-j.sh | 39 +++
tests/misc/tac-2-nonseekable.sh | 14 +++-
tests/misc/wc-proc.sh | 32 +
tests/split/b-chunk.sh | 39 +++
tests/tail-2/tail-c.sh | 35 ++
14 files changed, 463 insertions(+), 188 deletions(-)
create mode 100755 tests/misc/od-j.sh
create mode 100755 tests/misc/wc-proc.sh
create mode 100755 tests/tail-2/tail-c.sh
diff --git a/NEWS b/NEWS
index 1811ae4..a323b0c 100644
--- a/NEWS
+++ b/NEWS
@@ -12,6 +12,9 @@ GNU coreutils NEWS-*-
outline -*-
file types, a warning is issued for source directories with duplicate names,
or with -H the directory is copied again using the symlink name.
+ head, od, split, tac, tail, and wc no longer mishandle input from files in
+ /proc and /sys file systems that report somewhat-incorrect file sizes.
+
** New features
chroot accepts the new --skip-chdir option to not change the working
directory
diff --git a/src/head.c b/src/head.c
index d2f1fce..2782f8e 100644
--- a/src/head.c
+++ b/src/head.c
@@ -36,6 +36,7 @@
#include quote.h
#include quotearg.h
#include safe-read.h
+#include stat-size.h
#include xfreopen.h
#include xstrtol.h
@@ -206,13 +207,42 @@ copy_fd (int src_fd, uintmax_t n_bytes)
return COPY_FD_OK;
}
-/* Print all but the last N_ELIDE bytes from the input available via
- the non-seekable file descriptor FD. Return true upon success.
+/* Call lseek (FD, OFFSET, WHENCE), where file descriptor FD
+ corresponds to the file FILENAME. WHENCE must be SEEK_SET or
+ SEEK_CUR. Return the resulting offset. Give a diagnostic and
+ return -1 if lseek fails. */
+
+static off_t
+elseek (int fd, off_t offset, int whence, char const *filename)
+{
+ off_t new_offset = lseek (fd, offset, whence);
+ char buf[INT_BUFSIZE_BOUND (offset)];
+
+ if (new_offset 0)
+error (0, errno,
+ _(whence == SEEK_SET
+ ? N_(%s: cannot seek to offset %s)
+ : N_(%s: cannot seek to relative offset %s)),
+ quotearg_colon (filename),
+ offtostr (offset, buf));
+
+ return new_offset;
+}
+
+/* For an input file with name FILENAME and descriptor FD,
+ output all but the last N_ELIDE_0 bytes.
+ If CURRENT_POS is nonnegative, assume that the input file is
+ positioned at CURRENT_POS and that it should be repositioned to
+ just before the elided bytes before returning.
+ Return true upon success.
Give a diagnostic and return false upon error. */
static bool
-elide_tail_bytes_pipe (const char *filename, int fd, uintmax_t n_elide_0)
+elide_tail_bytes_pipe (const char *filename, int fd, uintmax_t
bug#18621: [BUG] wc -c incorrectly counts bytes in /sys
On 10/08/2014 12:51 AM, Paul Eggert wrote: Paul Eggert wrote: The attached patch still needs a changelog entry and test cases. I wrote those up and pushed the attached patch; this should fix the bug so I'm closing the bug report. I was just going through the patch as it happens, and I found no issues. I see the tac changes should also avoid erroneous errors if a file was truncated while reading for example. I pushed a follow up to avoid some new syntax check errors. thanks! Pádraig.
bug#8527: cp/mv in coreutils don't respect the default ACL of parent
Sorry, I didn't forward this to the right list...
The user data / extended attribute forks are where linux store the
ACL's. ext4 should
be configurable to do what you want to do, but I haven't personally used
it -- but
I understand it has similar functionality as xfs. The process umask is
a masking
off of privs/permissions one sets on a normal file (ACL's aside). It
affects the
permission bits on the file So if your umask was 077, then you open a
file for
rwx rwx rwx, it would mask off group and other allowing the permissions
to be
700 or rwx, --- ---. (I might have the order backwards, but it's the
standard
order you see in ls with numeric permissions)...Your umask will affect
your file
mode creation, but it depends on what flags you use when you use 'cp' --
which is one
of the main points of my detail... after everything was shown to be
working correctly
in my case, a setting I have in an alias to my cp would have
over-ridden any
other settings and made it look like 'cp' ignored directory ACL or
(sounds like you might
be talking group-owner ship -- of a dir -- or are you talking both).
Really, I'm not a member of the core utils devel group, so I really
prefer you send your
answers and questions there, as they'll catch alot more things than I
would -- I was
just showing an example of how your setting can override everything you
think you are
setting -- so you'll need to provide more detail about what your umask
is, (type umask at
prompt to see), and whether or not you have any aliases or ENV vars in
effect that could
alter things. If you can give an exact formula along the lines of what
I did to
demonstrate your problem, that will help the developers the most. The
detail I gave
was only to show how things you don't think of may be affecting you and
to be sure to check for them. I'm cc'ing the list on my reply, but
leaving your email off of it, so if you want to ask them if they need
more information that's fine... otherwise, write down the exact
commands you typed and your environment, to repeat it.. (umask included).
If you want to use my lsacl script.. it's a trivial build on top of the
chacl
program. But please post to the list so everyone can be on the same
page
lsacl script
more lsacl
#!/bin/bash
acllen=0
for fn in $@; do
out=$(chacl -l $fn)
qfn=$(printf %q $fn)
perm=${out#$qfn}
thislen=${#perm}
if ((thislenacllen)); then acllen=$thislen; fi
printf %-${acllen}s %s\n $perm $fn
done
=
Very trivial... but allowed me to look at multiple files at a time...
IF you can give a recipe or script that duplicates the problem you saw,
that would
be the best way to move this bug along (toward cockpit error or new
special case found!).
Best of luck either way!
bug#18621: [BUG] wc -c incorrectly counts bytes in /sys
On Tue, Oct 7, 2014 at 5:36 PM, Pádraig Brady p...@draigbrady.com wrote: On 10/08/2014 12:51 AM, Paul Eggert wrote: Paul Eggert wrote: The attached patch still needs a changelog entry and test cases. I wrote those up and pushed the attached patch; this should fix the bug so I'm closing the bug report. I was just going through the patch as it happens, and I found no issues. I see the tac changes should also avoid erroneous errors if a file was truncated while reading for example. I pushed a follow up to avoid some new syntax check errors. thanks! Pádraig. I pushed a patch to avoid a new failiure of the split/b-chunk.sh test on non-Linux.
