Skip to content

Commit eff8d69

Browse files
committed
Auto merge of #11724 - ehuss:cargo-ok-comment, r=weihanglo
Update comment about cargo-ok In #11665 I added some code to deal with a truncated `.cargo-ok` file. However I misunderstood the likely reason this was happening. Rust versions from 1.0 to 1.34 created empty files, and versions 1.36 to 1.49 could create empty files if they were interrupted during extraction. This PR updates the comment to try to explain this in detail to replace the erroneous comment from before. I think the change of deleting the directory is still the safe choice, even if it causes some cache churn when dealing with crates extracted by versions before 1.34.
2 parents 17b3d0d + c43136e commit eff8d69

File tree

1 file changed

+41
-3
lines changed
  • src/cargo/sources/registry

1 file changed

+41
-3
lines changed

src/cargo/sources/registry/mod.rs

Lines changed: 41 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -625,9 +625,47 @@ impl<'cfg> RegistrySource<'cfg> {
625625
match path.metadata() {
626626
Ok(meta) if meta.len() > 0 => return Ok(unpack_dir.to_path_buf()),
627627
Ok(_meta) => {
628-
// The file appears to be corrupted. Perhaps it failed to flush,
629-
// or the filesystem was corrupted in some way. To be safe, let's
630-
// assume something is wrong and clear it and start over.
628+
// The `.cargo-ok` file is not in a state we expect it to be
629+
// (with two bytes containing "ok").
630+
//
631+
// Cargo has always included a `.cargo-ok` file to detect if
632+
// extraction was interrupted, but it was originally empty.
633+
//
634+
// In 1.34, Cargo was changed to create the `.cargo-ok` file
635+
// before it started extraction to implement fine-grained
636+
// locking. After it was finished extracting, it wrote two
637+
// bytes to indicate it was complete. It would use the length
638+
// check to detect if it was possibly interrupted.
639+
//
640+
// In 1.36, Cargo changed to not use fine-grained locking, and
641+
// instead used a global lock. The use of `.cargo-ok` was no
642+
// longer needed for locking purposes, but was kept to detect
643+
// when extraction was interrupted.
644+
//
645+
// In 1.49, Cargo changed to not create the `.cargo-ok` file
646+
// before it started extraction to deal with `.crate` files
647+
// that inexplicably had a `.cargo-ok` file in them.
648+
//
649+
// In 1.64, Cargo changed to detect `.crate` files with
650+
// `.cargo-ok` files in them in response to CVE-2022-36113,
651+
// which dealt with malicious `.crate` files making
652+
// `.cargo-ok` a symlink causing cargo to write "ok" to any
653+
// arbitrary file on the filesystem it has permission to.
654+
//
655+
// This is all a long-winded way of explaining the
656+
// circumstances that might cause a directory to contain a
657+
// `.cargo-ok` file that is empty or otherwise corrupted.
658+
// Either this was extracted by a version of Rust before 1.34,
659+
// in which case everything should be fine. However, an empty
660+
// file created by versions 1.36 to 1.49 indicates that the
661+
// extraction was interrupted and that we need to start again.
662+
//
663+
// Another possibility is that the filesystem is simply
664+
// corrupted, in which case deleting the directory might be
665+
// the safe thing to do. That is probably unlikely, though.
666+
//
667+
// To be safe, this deletes the directory and starts over
668+
// again.
631669
log::warn!("unexpected length of {path:?}, clearing cache");
632670
paths::remove_dir_all(dst.as_path_unlocked())?;
633671
}

0 commit comments

Comments
 (0)