uutils / coreutils

Cross-platform Rust rewrite of the GNU coreutils
https://uutils.github.io/
MIT License
17.56k stars 1.26k forks source link

cat: wrong concatenation binary content #5186

Open d-enk opened 1 year ago

d-enk commented 1 year ago

$ cat -A /proc/1/cmdline /proc/version
/sbin/init^@splash^@Linux version 6.2.0-26-generic (buildd@bos03-amd64-042) (x86_64-linux-gnu-gcc-11 (Ubuntu 11.3.0-1ubuntu1~22.04.1) 11.3.0, GNU ld (GNU Binutils for Ubuntu) 2.38) #26~22.04.1-Ubuntu SMP PREEMPT_DYNAMIC Thu Jul 13 16:27:29 UTC 2$

$ coreutils cat -A /proc/1/cmdline /proc/version
/sbin/init^@splash^@Linux version 6.2.0-26-generic (buildd@bos03-amd64-042) (x86_64-linux-gnu-gcc-11 (Ubuntu 11.3.0-1ubuntu1~22.04.1) 11.3.0, GNU ld (GNU Binutils for Ubuntu) 2.38) #26~22.04.1-Ubuntu SMP PREEMPT_DYNAMIC Thu Jul 13 16:27:29 UTC 2$

But

$ coreutils cat /proc/1/cmdline /proc/version    
Linux version 6.2.0-26-generic (buildd@bos03-amd64-042) (x86_64-linux-gnu-gcc-11 (Ubuntu 11.3.0-1ubuntu1~22.04.1) 11.3.0, GNU ld (GNU Binutils for Ubuntu) 2.38) #26~22.04.1-Ubuntu SMP PREEMPT_DYNAMIC Thu Jul 13 16:27:29 UTC 2
/sbin/initsplash

$ cat /proc/1/cmdline /proc/version
/sbin/initsplashLinux version 6.2.0-26-generic (buildd@bos03-amd64-042) (x86_64-linux-gnu-gcc-11 (Ubuntu 11.3.0-1ubuntu1~22.04.1) 11.3.0, GNU ld (GNU Binutils for Ubuntu) 2.38) #26~22.04.1-Ubuntu SMP PREEMPT_DYNAMIC Thu Jul 13 16:27:29 UTC 2
tertsdiepraam commented 1 year ago

Something strange is going on here 🤔 It only happens in the write_fast branch. There's probably some misuse of splice or unistd::read or unistd::write there somewhere.

gim913 commented 1 year ago

It seems that in first call to splice() here (when dealing with /proc/1/cmdline) https://github.com/uutils/coreutils/blob/1adca622e9e1af4cbf2e6d520f7902131000dbf8/src/uu/cat/src/splice.rs#L30C48-L30C59

splice fails[*] and write_fast_using_splice exits with Ok(true) (so write_fast falls back to read + write),

there's a lock around stdout, however, lock won't cause stdout to flush, until newline appears... (and /proc/1/cmdline does not have newline)

now there comes next splice this time the one for /proc/version - this time it succeeds and output is written via splice_exact.

Finally something (likely process exiting?) flushes first write outputting /sbin/init

[*] - I haven't identified the cause of that failure.

This could be solved by adding explicit flush just before exiting here: https://github.com/uutils/coreutils/blob/1adca622e9e1af4cbf2e6d520f7902131000dbf8/src/uu/cat/src/cat.rs#L447

I'm not sure if that's the right fix though

tertsdiepraam commented 1 year ago

Oh right, that also explains why it's only newline-less content that causes this. flush sounds like a pretty good solution. Would you like to submit a PR for this?

gim913 commented 1 year ago

Here you go, I'm not sure if test makes sense / is good enough, I'm not quite sure how to replicate it in some nicer way. (can take a look tomorrow)

gim913 commented 1 year ago

I've managed to simplify the test a bit (by using alpha.txt), I think now it probably should work with #[cfg(unix)] (not 100% sure, but I think all nix flavors use /sbin/init, unless overridden by boot options).

I was also trying with some short 0-ended file (instead of using /proc/1/cmdline), but couldn't get splice() to fail.