std::fs::Metadata timestamp methods will panic on files with invalid timestamp nsec values #108277
Comments
|
@rustbot claim |
Noratrieb
added
the
T-libs
You mean that created return Err ? I agree, this shouldn't panic. |
Ah yeah right, this is a |
|
Any news on this? |
|
This should be a priority. Programs written in Rust simply crash without this fix. |
|
I will see what I can do @rustbot claim |
|
I did a few work on this, there is few thing to consider, while remove the assert would remove the panic, the current proposition here say to return a error instead this mean that program will still need to handle the error, there is another possibility is to silent to error by rounding the value, like for example add extra time from nanosecond to second and so remove the weird nanosecond value. I don't have strong opinion on this. |
|
Adding the nanoseconds is probably not a very good idea. glibc defines tv_nsec as long int, which is 64 bits on amd64. That's still up to a ±292 years offset. If an error is undesirable I recommend setting it to 0 instead. In the case that only nsec is corrupted and not the seconds too this would at least keep the seconds correct. |
|
In this case the seconds are corrupted too. 5006491383123099853 is well over a billion years in the future. I had to remove the last two digits to get |
|
I think invalid data should be handled as an error and not interpreted. |
The solution depends on the situation at hand. In this case, if an error arises, the value should be set to zero. There's no need for rounding or interpretation - simply zero. Why? Because we're dealing with a computer language. This issue impacts all systems possessing files with invalid timestamps within the file system. Indeed, even the kernel reads these timestamps without triggering an error or causing a crash. |
|
I think I prefer we return an error, for now I used Then, the next problem is how does one solve the error using Rust ? Do we want std to include a way to "set" the creation time of a file ? is that even possible ? Also, one last thing, does anyone how a way I could add a test for this ? |
|
I encountered the same issue on MacOS 13.5.1 (on APFS), and found it to be related to nanoseconds set to any value before unix epoch. Obtaining the However, Interestingly, |
|
When a filesystem is returning invalid date data, producing an InvalidData error seems like a reasonable way to handle this, rather than panicking. That will let the program skip that piece of the file's information and move on. (And, in the case of a tool that isn't actually displaying that particular piece of information, that shouldn't affect the output.) We should also document that this error can occur ("If the filesystem returns invalid timestamp information, this will return an |
|
Correction after some further investigation with @Byron: turns out the issue on macOS is entirely different than the Linux issue, and the issue on macOS can be easily corrected in std. Longer write-up shortly. |
|
Full details on the macOS bug: Time in UNIX system calls counts from the epoch, 1970-01-01. The So, seconds=0 means 1970-01-01, seconds=86400 means 1970-01-02, etc. Suppose you ask for the time 1969-12-31, what time is that? On UNIX systems that support times before the epoch, that's seconds=-86400, one day before the epoch. Perfectly reasonable so far. But now, suppose you ask for the time 1969-12-31 23:59:00.1. In other words, a tenth of a second after one minute before the epoch. On most UNIX systems, that's represented as seconds=-60, nanoseconds=100_000_000. The macOS bug is that it returns seconds=-59, nanoseconds=-900_000_000. I can see how they got that: it's 59.9 seconds before the epoch. But that violates the invariant of the timespec data structure: nanoseconds must be between 0 and 999999999. Which, in turn, causes this assertion in the Rust standard library. We can fix this, though: on macOS, if we get a Timespec value with seconds less than or equal to zero, and nanoseconds between -999_999_999 and -1 (inclusive), we can add 1_000_000_000 to the nanoseconds and subtract 1 from the seconds, and then convert. The resulting timespec value is still accepted by macOS, and when fed back into the OS, produces the same results. (If you set a file's mtime with that timestamp, then read it back, you get back the one with negative nanoseconds again.) |
|
Was there a consensus on how to proceed with this for non-Apple platforms? It seems to be cropping up fairly regularly in |
|
@daviessm For non-macOS platforms, where the issue is a bad directory rather than an OS bug, I think we should be detecting the invalid timestamp and returning an InvalidData error rather than panicking. That should make it possible to handle the error and display a placeholder / error indication. |
|
@joshtriplett I'd agree with that from the |
|
I think this would then have to mean that the error would have to be returned by one of the methods return Maybe for even greater usability, a new constructor for |
As @Byron noted, the three timestamp methods of Metadata return |
|
Yes, sorry, I was thinking about the |
They seem to return I encourage everyone reading this to consider the issue a great opportunity for making a contribution to Rust, just judging from my experience fixing the bug on MacOS. It took only about 30 minutes to get into the red-green-refactor cycle, and from there it basically fixed itself. The only problem I'd have implementing this is the lack of reproducible test-case - how would it be possible to even get a file with a broken filestamp? One would probably have to hex-edit the binary blob of an extracted filesystem to get there and then mount it. Alternatively, maybe it's enough to not break an existing test when implementing the fix, along with a particularly thorough review of multiple parties. |
Would FUSE work? Or does it do a sanity check of the timestamps? Or |
|
Just my piece of info that may be relevant: It either some sort of upstream/kernel/btrfs(which I'm using) bug or files/directories with such birthtimes are just normal PS. When eza attempts to work with those directories, rust panics, what is shown in the issue above my comment Interesting part is that for me stat shows |
You are using GNU coreutils which shows Edit: Looks like GNU coreutils uses |
|
|
@bjorn3 why if it returns Also what do other linux people with or without btrfs have as their stat of |
Because https://github.com/coreutils/coreutils/blob/a966dcdb69e2f49f2587e1b7d4ade7efcff29f40/src/stat.c#L1617 is the code that checks for negative values and prints |
I don't use macos. I have never stated that I use macos. I don't understand where this is coming from. I use Linux with btrfs, which is where the corruption comes from. But I don't see how this is relevant. The broken timestamps are there, and there will always be cases where those or similar problems will exist. This is not LKML and I would consider it quite offtopic to be debugging filesystem bugs here. The only thing we can do here is mitigate them. |
My bad. Must have gotten confused by the macOS mention at #108277 (comment). |
|
Sorry, @felinira I was confused about macOS because @bjorn3 were confused about it 😆 I understand that this is not LKML, but i asked my question because all of the people that came here have something in common. And they are here. On github. Accessible. I'm just trying to get most likely possible reason why this bug (not that rust panics but the fact there are files with negative or broken birthtimes) happened before reporting it in place it belongs. And what's handy people that came here most likely have also those broken files/directories. If most of the people here also report for example that reading files in / fails because of the /sys /proc dirs this also may be an important detail to report to LKML rather than just shoot into the air with my bug report |
|
I'm using Arch Linux with BTRFS and with nushell v0.89. When I run stat ~/.config/ File: /home/user/.config/
Size: 3142 Blocks: 0 IO Block: 4096 directory
Device: 0,39 Inode: 4010908 Links: 1
Access: (0755/drwxr-xr-x) Uid: ( 1000/ user) Gid: ( 1001/ user)
Access: 2022-08-29 18:00:58.782805198 0300
Modify: 2024-01-17 13:17:41.297652651 0200
Change: 2024-01-17 13:17:41.297652651 0200
Birth: 6012152045059794253.1935959397All other files in my $HOME have a normal Birth time. Also running with the builtin ls |
|
Here's a command to identify files/directories with bogus birth times: sudo find "$HOME" -print0 \
| sudo xargs -0 ls -ld --time=birth \
| rg '^(\S \s ){5}[0-9] /'There are 69 on my machine. They seem to be concentrated in places that would see frequent metadata updates. SQLite databases and their -wal files, directories with frequent creation/deletion of files, etc. The exception, is a cluster of files with names like Then outside the Presumably there is some condition of system load that makes the corruption more likely, or particularly sensitive kernel versions. Maybe temperature, line voltage... Anyhow, this is how I got the atimes for the culprits: ls -ltrhd --time=atime --time-style=full-iso (sed -E 's/^(\S \s ){6}//' culprits.txt)Just a hunch, but since birth time is a recent addition to the file attributes, maybe it's on the end of the buffer? |
m-ou-se
added
the
I-libs-nominated
m-ou-se
removed
the
I-libs-nominated
|
We discussed this in today's @rust-lang/libs meeting, and we agreed that we should just return an |
unix time module now return result First try to fix rust-lang#108277 without break anything. if anyone who read this know tips to be able to check compilation for different target I could use some help. So far I installed many target with rustup but `./x check --all-targets` doesn't seem to use them. TODO: - [x] better error - [ ] test, how ? `@rustbot` label -S-waiting-on-author S-waiting-on-review
unix time module now return result First try to fix rust-lang#108277 without break anything. if anyone who read this know tips to be able to check compilation for different target I could use some help. So far I installed many target with rustup but `./x check --all-targets` doesn't seem to use them. TODO: - [x] better error - [ ] test, how ? ``@rustbot`` label -S-waiting-on-author S-waiting-on-review
std::fs::Metadatatimestamp methods (likecreated) will panic on files with invalid timestamp nsec values.`It will panic here:
thread 'async-std/runtime' panicked at 'assertion failed: tv_nsec >= 0 && tv_nsec < NSEC_PER_SEC as i64', library/std/src/sys/unix/time.rs:66:9We have recently stumbled upon multiple btrfs users that happen to have files that look something like this:
This is most likely a filesystem / kernel bug. But these files / broken kernels exist now and will cause any access to
ctime(and possible the other time values) to panic.My proposal would be to check bounds of the underlying syscall results and return an appropriate
Errfor these broken time values if the ctime / mtime / atime etc. contains values that would otherwise trigger this assertion.Meta
rustc --version --verbose:Backtrace
The text was updated successfully, but these errors were encountered: