wasi: do not panic on extreme file timestamps in DescriptorStat#13894
wasi: do not panic on extreme file timestamps in DescriptorStat#13894SebTardif wants to merge 4 commits into
Conversation
Datetime::try_from can fail when SystemTime is outside the i64-second range. datetime_from used unwrap(), so a guest that set extreme timestamps on a preopened path could crash the host on the next stat. Map conversion failure to a missing timestamp (Option::None) instead. Signed-off-by: Sebastien Tardif <sebtardif@ncf.ca>
Signed-off-by: Sebastien Tardif <sebtardif@ncf.ca>
alexcrichton
left a comment
alexcrichton
left a comment
There was a problem hiding this comment.
Thanks for the PR, but please be sure you review our AI tool usage policy. You own the code and you own the responsibility for being correct here, and for example the analysis of when this was introduced was wrong. The PR referred there was a refactor and did not introduce this.
Inline try_from mapping, drop narrative comments, and keep a small unit test under mod test for SystemTime values outside Datetime's range. Guest set_times cannot construct those values because they already use Datetime; host Metadata is the overflow path. Signed-off-by: Sebastien Tardif <sebtardif@ncf.ca>
Thanks for the review. I re-read the AI tool use policy and corrected the PR description: #13872 was a refactor of the conversion site, not the introduction of this failure mode. Sorry for the incorrect origin claim. Addressed the inline notes in the latest commit: inlined the conversion, |
Prepare a host file with an extreme mtime and have the guest path_filestat_get and read it (p1 core module and p2 component runners). Also treat wall-clock conversions that fail for negative/out-of-range i64 timestamps as missing fields so the whole stat does not return OVERFLOW on platforms that clamp far- past times into i64. Signed-off-by: Sebastien Tardif <sebtardif@ncf.ca>
2 participants
Summary
DescriptorStat::newconverted hostSystemTimevalues to WASIDatetimewithunwrap(). When conversion fails (time outside the representablei64second range), that panics the host while building file stats.Map conversion failure to
None(missing timestamp) on the existingOption<Datetime>fields.Failure scenario
Host
Metadatacan surfaceSystemTimevalues that do not fit WASIDatetime(for example extreme far-past times representable asSystemTimebut not asi64seconds). A subsequent guest or hoststatpath that buildsDescriptorStatwould panic instead of returning stats with missing timestamp fields.Guest
set_timesis not the reproducer for the overflow itself: those APIs already takeDatetimevalues within range. The panic is on host-side conversion ofSystemTimeintoDatetime.Origin
The previous description incorrectly attributed introduction to the cap-primitives migration (#13872). That PR refactored the conversion site; the unwrap-style failure mode is not claimed as newly introduced there. This change is a small hardening of the conversion.
Test
Unit test under
crates/wasi/src/filesystem.rs(mod test) asserts that extremeSystemTimevalues failDatetime::try_fromand map toNonewithout panicking.Integration coverage of the surrounding set_times/stat stack already exists as
p1_fd_filestat_setincrates/test-programs/crates/wasi/tests. A guest program cannot construct the overflowingSystemTimevia the public WASI timestamp APIs, so the edge case stays as a unit test on the conversion.