From b16715751a222be1953bae1010c77eae6996fd8a Mon Sep 17 00:00:00 2001 From: Caleb Cartwright Date: Fri, 14 Jan 2022 18:18:37 -0600 Subject: [PATCH 01/15] fix(rustfmt): resolve generated file formatting issue --- src/tools/rustfmt/Configurations.md | 4 ++-- src/tools/rustfmt/src/config/mod.rs | 4 ++-- src/tools/rustfmt/src/formatting.rs | 2 +- src/tools/rustfmt/src/test/mod.rs | 18 ++++++++++++++++++ 4 files changed, 23 insertions(+), 5 deletions(-) diff --git a/src/tools/rustfmt/Configurations.md b/src/tools/rustfmt/Configurations.md index 13826883d2f4b..04c8429e5fd1c 100644 --- a/src/tools/rustfmt/Configurations.md +++ b/src/tools/rustfmt/Configurations.md @@ -929,9 +929,9 @@ fn add_one(x: i32) -> i32 { ## `format_generated_files` Format generated files. A file is considered generated -if any of the first five lines contains `@generated` marker. +if any of the first five lines contain a `@generated` comment marker. -- **Default value**: `false` +- **Default value**: `true` - **Possible values**: `true`, `false` - **Stable**: No diff --git a/src/tools/rustfmt/src/config/mod.rs b/src/tools/rustfmt/src/config/mod.rs index c5419d860c943..0aabfceb5e8cf 100644 --- a/src/tools/rustfmt/src/config/mod.rs +++ b/src/tools/rustfmt/src/config/mod.rs @@ -138,7 +138,7 @@ create_config! { inline_attribute_width: usize, 0, false, "Write an item and its attribute on the same line \ if their combined width is below a threshold"; - format_generated_files: bool, false, false, "Format generated files"; + format_generated_files: bool, true, false, "Format generated files"; // Options that can change the source code beyond whitespace/blocks (somewhat linty things) merge_derives: bool, true, true, "Merge multiple `#[derive(...)]` into a single one"; @@ -608,7 +608,7 @@ blank_lines_lower_bound = 0 edition = "2015" version = "One" inline_attribute_width = 0 -format_generated_files = false +format_generated_files = true merge_derives = true use_try_shorthand = false use_field_init_shorthand = false diff --git a/src/tools/rustfmt/src/formatting.rs b/src/tools/rustfmt/src/formatting.rs index 7d0facb8f12cf..05a74d322b359 100644 --- a/src/tools/rustfmt/src/formatting.rs +++ b/src/tools/rustfmt/src/formatting.rs @@ -109,7 +109,7 @@ fn format_project( let src = source_file.src.as_ref().expect("SourceFile without src"); let should_ignore = (!input_is_stdin && context.ignore_file(&path)) - || (!config.format_generated_files() && is_generated_file(src)); + || (!input_is_stdin && !config.format_generated_files() && is_generated_file(src)); if (config.skip_children() && path != main_file) || should_ignore { continue; diff --git a/src/tools/rustfmt/src/test/mod.rs b/src/tools/rustfmt/src/test/mod.rs index e2620508c340b..f7ae8c6b5b10f 100644 --- a/src/tools/rustfmt/src/test/mod.rs +++ b/src/tools/rustfmt/src/test/mod.rs @@ -490,6 +490,24 @@ fn stdin_disable_all_formatting_test() { assert_eq!(input, String::from_utf8(output.stdout).unwrap()); } +#[test] +fn stdin_generated_files_issue_5172() { + init_log(); + let input = Input::Text("//@generated\nfn main() {}".to_owned()); + let mut config = Config::default(); + config.set().emit_mode(EmitMode::Stdout); + config.set().format_generated_files(false); + config.set().newline_style(NewlineStyle::Unix); + let mut buf: Vec = vec![]; + { + let mut session = Session::new(config, Some(&mut buf)); + session.format(input).unwrap(); + assert!(session.has_no_errors()); + } + // N.B. this should be changed once `format_generated_files` is supported with stdin + assert_eq!(buf, "stdin:\n\n//@generated\nfn main() {}\n".as_bytes()); +} + #[test] fn format_lines_errors_are_reported() { init_log(); From 70a61c97cbc8375432b16482f127d45b08054951 Mon Sep 17 00:00:00 2001 From: Aaron Hill Date: Fri, 26 Nov 2021 09:14:16 -0600 Subject: [PATCH 02/15] Only check for errors in predicate when skipping impl assembly Prior to PR #91205, checking for errors in the overall obligation would check checking the `ParamEnv`, due to an incorrect `super_visit_with` impl. With this bug fixed, we will now bail out of impl candidate assembly if the `ParamEnv` contains any error types. In practice, this appears to be overly conservative - when an error occurs early in compilation, we end up giving up early for some predicates that we could have successfully evaluated without overflow. By only checking for errors in the predicate itself, we avoid causing additional spurious 'type annotations needed' errors after a 'real' error has already occurred. With this PR, the diagnostic changes caused by PR #91205 are reverted. --- .../src/traits/select/candidate_assembly.rs | 5 ++++- .../issue-72787.min.stderr | 22 ++++++------------- .../generic_const_exprs/issue-72787.rs | 1 - src/test/ui/issues/issue-77919.rs | 2 +- src/test/ui/issues/issue-77919.stderr | 15 +++++++------ .../clippy/tests/ui/crashes/ice-6252.stderr | 15 +++++++------ 6 files changed, 28 insertions(+), 32 deletions(-) diff --git a/compiler/rustc_trait_selection/src/traits/select/candidate_assembly.rs b/compiler/rustc_trait_selection/src/traits/select/candidate_assembly.rs index 0ff3611f8f80d..6e3e3b9b14480 100644 --- a/compiler/rustc_trait_selection/src/traits/select/candidate_assembly.rs +++ b/compiler/rustc_trait_selection/src/traits/select/candidate_assembly.rs @@ -536,7 +536,10 @@ impl<'cx, 'tcx> SelectionContext<'cx, 'tcx> { // This helps us avoid overflow: see issue #72839 // Since compilation is already guaranteed to fail, this is just // to try to show the 'nicest' possible errors to the user. - if obligation.references_error() { + // We don't check for errors in the `ParamEnv` - in practice, + // it seems to cause us to be overly aggressive in deciding + // to give up searching for candidates, leading to spurious errors. + if obligation.predicate.references_error() { return; } diff --git a/src/test/ui/const-generics/generic_const_exprs/issue-72787.min.stderr b/src/test/ui/const-generics/generic_const_exprs/issue-72787.min.stderr index 3a7095789336c..02dce4f7a97e8 100644 --- a/src/test/ui/const-generics/generic_const_exprs/issue-72787.min.stderr +++ b/src/test/ui/const-generics/generic_const_exprs/issue-72787.min.stderr @@ -1,5 +1,5 @@ error: generic parameters may not be used in const operations - --> $DIR/issue-72787.rs:12:17 + --> $DIR/issue-72787.rs:11:17 | LL | Condition<{ LHS <= RHS }>: True | ^^^ cannot perform const operation using `LHS` @@ -8,7 +8,7 @@ LL | Condition<{ LHS <= RHS }>: True = help: use `#![feature(generic_const_exprs)]` to allow generic const expressions error: generic parameters may not be used in const operations - --> $DIR/issue-72787.rs:12:24 + --> $DIR/issue-72787.rs:11:24 | LL | Condition<{ LHS <= RHS }>: True | ^^^ cannot perform const operation using `RHS` @@ -17,7 +17,7 @@ LL | Condition<{ LHS <= RHS }>: True = help: use `#![feature(generic_const_exprs)]` to allow generic const expressions error: generic parameters may not be used in const operations - --> $DIR/issue-72787.rs:26:25 + --> $DIR/issue-72787.rs:25:25 | LL | IsLessOrEqual<{ 8 - I }, { 8 - J }>: True, | ^ cannot perform const operation using `I` @@ -26,7 +26,7 @@ LL | IsLessOrEqual<{ 8 - I }, { 8 - J }>: True, = help: use `#![feature(generic_const_exprs)]` to allow generic const expressions error: generic parameters may not be used in const operations - --> $DIR/issue-72787.rs:26:36 + --> $DIR/issue-72787.rs:25:36 | LL | IsLessOrEqual<{ 8 - I }, { 8 - J }>: True, | ^ cannot perform const operation using `J` @@ -35,15 +35,7 @@ LL | IsLessOrEqual<{ 8 - I }, { 8 - J }>: True, = help: use `#![feature(generic_const_exprs)]` to allow generic const expressions error[E0283]: type annotations needed - --> $DIR/issue-72787.rs:10:38 - | -LL | impl True for IsLessOrEqual where - | ^^^^ cannot infer type for struct `IsLessOrEqual` - | - = note: cannot satisfy `IsLessOrEqual: True` - -error[E0283]: type annotations needed - --> $DIR/issue-72787.rs:22:26 + --> $DIR/issue-72787.rs:21:26 | LL | IsLessOrEqual: True, | ^^^^ cannot infer type for struct `IsLessOrEqual` @@ -51,13 +43,13 @@ LL | IsLessOrEqual: True, = note: cannot satisfy `IsLessOrEqual: True` error[E0283]: type annotations needed - --> $DIR/issue-72787.rs:22:26 + --> $DIR/issue-72787.rs:21:26 | LL | IsLessOrEqual: True, | ^^^^ cannot infer type for struct `IsLessOrEqual` | = note: cannot satisfy `IsLessOrEqual: True` -error: aborting due to 7 previous errors +error: aborting due to 6 previous errors For more information about this error, try `rustc --explain E0283`. diff --git a/src/test/ui/const-generics/generic_const_exprs/issue-72787.rs b/src/test/ui/const-generics/generic_const_exprs/issue-72787.rs index 2ea5d634f6ef3..77ad57f0640fa 100644 --- a/src/test/ui/const-generics/generic_const_exprs/issue-72787.rs +++ b/src/test/ui/const-generics/generic_const_exprs/issue-72787.rs @@ -8,7 +8,6 @@ pub struct Condition; pub trait True {} impl True for IsLessOrEqual where -//[min]~^ ERROR type annotations needed Condition<{ LHS <= RHS }>: True //[min]~^ Error generic parameters may not be used in const operations //[min]~| Error generic parameters may not be used in const operations diff --git a/src/test/ui/issues/issue-77919.rs b/src/test/ui/issues/issue-77919.rs index 6e597b7669abc..966d76d148af3 100644 --- a/src/test/ui/issues/issue-77919.rs +++ b/src/test/ui/issues/issue-77919.rs @@ -10,4 +10,4 @@ struct Multiply { } impl TypeVal for Multiply where N: TypeVal {} //~^ ERROR cannot find type `VAL` in this scope -//~| ERROR type annotations needed +//~| ERROR not all trait items implemented, missing: `VAL` diff --git a/src/test/ui/issues/issue-77919.stderr b/src/test/ui/issues/issue-77919.stderr index f98556bc72f47..97bd5ab36b65d 100644 --- a/src/test/ui/issues/issue-77919.stderr +++ b/src/test/ui/issues/issue-77919.stderr @@ -17,15 +17,16 @@ LL | impl TypeVal for Multiply where N: TypeVal {} | | | help: you might be missing a type parameter: `, VAL` -error[E0283]: type annotations needed - --> $DIR/issue-77919.rs:11:12 +error[E0046]: not all trait items implemented, missing: `VAL` + --> $DIR/issue-77919.rs:11:1 | +LL | const VAL: T; + | ------------- `VAL` from trait +... LL | impl TypeVal for Multiply where N: TypeVal {} - | ^^^^^^^^^^^^^^ cannot infer type for struct `Multiply` - | - = note: cannot satisfy `Multiply: TypeVal` + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ missing `VAL` in implementation error: aborting due to 3 previous errors -Some errors have detailed explanations: E0283, E0412. -For more information about an error, try `rustc --explain E0283`. +Some errors have detailed explanations: E0046, E0412. +For more information about an error, try `rustc --explain E0046`. diff --git a/src/tools/clippy/tests/ui/crashes/ice-6252.stderr b/src/tools/clippy/tests/ui/crashes/ice-6252.stderr index abca7af30a006..c8239897f3abb 100644 --- a/src/tools/clippy/tests/ui/crashes/ice-6252.stderr +++ b/src/tools/clippy/tests/ui/crashes/ice-6252.stderr @@ -21,15 +21,16 @@ LL | impl TypeVal for Multiply where N: TypeVal {} | | | help: you might be missing a type parameter: `, VAL` -error[E0283]: type annotations needed - --> $DIR/ice-6252.rs:10:12 +error[E0046]: not all trait items implemented, missing: `VAL` + --> $DIR/ice-6252.rs:10:1 | +LL | const VAL: T; + | ------------- `VAL` from trait +... LL | impl TypeVal for Multiply where N: TypeVal {} - | ^^^^^^^^^^^^^^ cannot infer type for struct `Multiply` - | - = note: cannot satisfy `Multiply: TypeVal` + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ missing `VAL` in implementation error: aborting due to 3 previous errors -Some errors have detailed explanations: E0283, E0412. -For more information about an error, try `rustc --explain E0283`. +Some errors have detailed explanations: E0046, E0412. +For more information about an error, try `rustc --explain E0046`. From a14ae6f0791f499a2205a11846fdd75f25a5ae9a Mon Sep 17 00:00:00 2001 From: Yechan Bae Date: Wed, 19 Jan 2022 10:34:12 +0100 Subject: [PATCH 03/15] Move `non_send_fields_in_send_ty` back to `nursery` --- src/tools/clippy/clippy_lints/src/lib.register_all.rs | 1 - src/tools/clippy/clippy_lints/src/lib.register_nursery.rs | 1 + src/tools/clippy/clippy_lints/src/lib.register_suspicious.rs | 1 - src/tools/clippy/clippy_lints/src/non_send_fields_in_send_ty.rs | 2 +- 4 files changed, 2 insertions(+), 3 deletions(-) diff --git a/src/tools/clippy/clippy_lints/src/lib.register_all.rs b/src/tools/clippy/clippy_lints/src/lib.register_all.rs index 15edb79d36c24..8dac588b5162c 100644 --- a/src/tools/clippy/clippy_lints/src/lib.register_all.rs +++ b/src/tools/clippy/clippy_lints/src/lib.register_all.rs @@ -217,7 +217,6 @@ store.register_group(true, "clippy::all", Some("clippy_all"), vec![ LintId::of(non_copy_const::DECLARE_INTERIOR_MUTABLE_CONST), LintId::of(non_expressive_names::JUST_UNDERSCORES_AND_DIGITS), LintId::of(non_octal_unix_permissions::NON_OCTAL_UNIX_PERMISSIONS), - LintId::of(non_send_fields_in_send_ty::NON_SEND_FIELDS_IN_SEND_TY), LintId::of(open_options::NONSENSICAL_OPEN_OPTIONS), LintId::of(option_env_unwrap::OPTION_ENV_UNWRAP), LintId::of(overflow_check_conditional::OVERFLOW_CHECK_CONDITIONAL), diff --git a/src/tools/clippy/clippy_lints/src/lib.register_nursery.rs b/src/tools/clippy/clippy_lints/src/lib.register_nursery.rs index 44c75a11eec08..1e54482a8dafd 100644 --- a/src/tools/clippy/clippy_lints/src/lib.register_nursery.rs +++ b/src/tools/clippy/clippy_lints/src/lib.register_nursery.rs @@ -17,6 +17,7 @@ store.register_group(true, "clippy::nursery", Some("clippy_nursery"), vec![ LintId::of(missing_const_for_fn::MISSING_CONST_FOR_FN), LintId::of(mutable_debug_assertion::DEBUG_ASSERT_WITH_MUT_CALL), LintId::of(mutex_atomic::MUTEX_INTEGER), + LintId::of(non_send_fields_in_send_ty::NON_SEND_FIELDS_IN_SEND_TY), LintId::of(nonstandard_macro_braces::NONSTANDARD_MACRO_BRACES), LintId::of(option_if_let_else::OPTION_IF_LET_ELSE), LintId::of(path_buf_push_overwrite::PATH_BUF_PUSH_OVERWRITE), diff --git a/src/tools/clippy/clippy_lints/src/lib.register_suspicious.rs b/src/tools/clippy/clippy_lints/src/lib.register_suspicious.rs index a3f964d158042..8859787fbc830 100644 --- a/src/tools/clippy/clippy_lints/src/lib.register_suspicious.rs +++ b/src/tools/clippy/clippy_lints/src/lib.register_suspicious.rs @@ -15,7 +15,6 @@ store.register_group(true, "clippy::suspicious", Some("clippy_suspicious"), vec! LintId::of(loops::MUT_RANGE_BOUND), LintId::of(methods::SUSPICIOUS_MAP), LintId::of(mut_key::MUTABLE_KEY_TYPE), - LintId::of(non_send_fields_in_send_ty::NON_SEND_FIELDS_IN_SEND_TY), LintId::of(suspicious_trait_impl::SUSPICIOUS_ARITHMETIC_IMPL), LintId::of(suspicious_trait_impl::SUSPICIOUS_OP_ASSIGN_IMPL), ]) diff --git a/src/tools/clippy/clippy_lints/src/non_send_fields_in_send_ty.rs b/src/tools/clippy/clippy_lints/src/non_send_fields_in_send_ty.rs index 7ebf84d400f56..374b7bd59649e 100644 --- a/src/tools/clippy/clippy_lints/src/non_send_fields_in_send_ty.rs +++ b/src/tools/clippy/clippy_lints/src/non_send_fields_in_send_ty.rs @@ -44,7 +44,7 @@ declare_clippy_lint! { /// Use thread-safe types like [`std::sync::Arc`](https://doc.rust-lang.org/std/sync/struct.Arc.html) /// or specify correct bounds on generic type parameters (`T: Send`). pub NON_SEND_FIELDS_IN_SEND_TY, - suspicious, + nursery, "there is field that does not implement `Send` in a `Send` struct" } From 408709d88c4e8928aaf57663e6d4b692839e9626 Mon Sep 17 00:00:00 2001 From: Jason Newcomb Date: Wed, 19 Jan 2022 10:35:22 +0100 Subject: [PATCH 04/15] Handle implicit named arguments in `useless_format` --- src/tools/clippy/clippy_lints/src/format.rs | 19 +++++++++++++++++-- src/tools/clippy/tests/ui/format.fixed | 6 ++++++ src/tools/clippy/tests/ui/format.rs | 6 ++++++ src/tools/clippy/tests/ui/format.stderr | 14 +++++++++++++- 4 files changed, 42 insertions(+), 3 deletions(-) diff --git a/src/tools/clippy/clippy_lints/src/format.rs b/src/tools/clippy/clippy_lints/src/format.rs index 7169ac9ad6c5a..c20534672026b 100644 --- a/src/tools/clippy/clippy_lints/src/format.rs +++ b/src/tools/clippy/clippy_lints/src/format.rs @@ -9,7 +9,7 @@ use rustc_lint::{LateContext, LateLintPass}; use rustc_middle::ty; use rustc_session::{declare_lint_pass, declare_tool_lint}; use rustc_span::symbol::kw; -use rustc_span::{sym, Span}; +use rustc_span::{sym, BytePos, Span}; declare_clippy_lint! { /// ### What it does @@ -80,7 +80,22 @@ impl<'tcx> LateLintPass<'tcx> for UselessFormat { ExprKind::MethodCall(path, ..) => path.ident.name.as_str() == "to_string", _ => false, }; - let sugg = if is_new_string { + let sugg = if format_args.format_string_span.contains(value.span) { + // Implicit argument. e.g. `format!("{x}")` span points to `{x}` + let spdata = value.span.data(); + let span = Span::new( + spdata.lo + BytePos(1), + spdata.hi - BytePos(1), + spdata.ctxt, + spdata.parent + ); + let snip = snippet_with_applicability(cx, span, "..", &mut applicability); + if is_new_string { + snip.into() + } else { + format!("{}.to_string()", snip) + } + } else if is_new_string { snippet_with_applicability(cx, value.span, "..", &mut applicability).into_owned() } else { let sugg = Sugg::hir_with_applicability(cx, value, "", &mut applicability); diff --git a/src/tools/clippy/tests/ui/format.fixed b/src/tools/clippy/tests/ui/format.fixed index 64cb7b1cfb80f..78d2bfd474e4a 100644 --- a/src/tools/clippy/tests/ui/format.fixed +++ b/src/tools/clippy/tests/ui/format.fixed @@ -73,4 +73,10 @@ fn main() { let _s: String = (&*v.join("\n")).to_string(); format!("prepend {:+}", "s"); + + // Issue #8290 + let x = "foo"; + let _ = x.to_string(); + let _ = format!("{x:?}"); // Don't lint on debug + let _ = x.to_string(); } diff --git a/src/tools/clippy/tests/ui/format.rs b/src/tools/clippy/tests/ui/format.rs index a065b1b5683c1..009c1aa216fcd 100644 --- a/src/tools/clippy/tests/ui/format.rs +++ b/src/tools/clippy/tests/ui/format.rs @@ -75,4 +75,10 @@ fn main() { let _s: String = format!("{}", &*v.join("\n")); format!("prepend {:+}", "s"); + + // Issue #8290 + let x = "foo"; + let _ = format!("{x}"); + let _ = format!("{x:?}"); // Don't lint on debug + let _ = format!("{y}", y = x); } diff --git a/src/tools/clippy/tests/ui/format.stderr b/src/tools/clippy/tests/ui/format.stderr index 58ad7499bb26f..660be57585e37 100644 --- a/src/tools/clippy/tests/ui/format.stderr +++ b/src/tools/clippy/tests/ui/format.stderr @@ -99,5 +99,17 @@ error: useless use of `format!` LL | let _s: String = format!("{}", &*v.join("/n")); | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: consider using `.to_string()`: `(&*v.join("/n")).to_string()` -error: aborting due to 15 previous errors +error: useless use of `format!` + --> $DIR/format.rs:81:13 + | +LL | let _ = format!("{x}"); + | ^^^^^^^^^^^^^^ help: consider using `.to_string()`: `x.to_string()` + +error: useless use of `format!` + --> $DIR/format.rs:83:13 + | +LL | let _ = format!("{y}", y = x); + | ^^^^^^^^^^^^^^^^^^^^^ help: consider using `.to_string()`: `x.to_string()` + +error: aborting due to 17 previous errors From acd647611d03cc80bb2b9cc6186c8f83c8ca7940 Mon Sep 17 00:00:00 2001 From: Pietro Albini Date: Wed, 19 Jan 2022 10:50:06 +0100 Subject: [PATCH 05/15] add release notes for 1.58.1 --- RELEASES.md | 13 +++++++++++++ 1 file changed, 13 insertions(+) diff --git a/RELEASES.md b/RELEASES.md index bb2b9c2cea134..8c358a2879289 100644 --- a/RELEASES.md +++ b/RELEASES.md @@ -1,3 +1,16 @@ +Version 1.58.1 (2022-01-19) +=========================== + +* [Handle captured arguments in the `useless_format` Clippy lint][clippy/8295] +* [Move `non_send_fields_in_send_ty` Clippy lint to nursery][clippy/8075] +* [Fix wrong error message displayed when some imports are missing][91254] +* [Fix rustfmt not formatting generated files from stdin][92912] + +[91254]: https://github.com/rust-lang/rust/pull/91254 +[92912]: https://github.com/rust-lang/rust/pull/92912 +[clippy/8075]: https://github.com/rust-lang/rust-clippy/pull/8075 +[clippy/8295]: https://github.com/rust-lang/rust-clippy/pull/8295 + Version 1.58.0 (2022-01-13) ========================== From 0076b17beffa3aca955b5311fcdeb2a1309af865 Mon Sep 17 00:00:00 2001 From: Pietro Albini Date: Wed, 19 Jan 2022 17:19:33 +0100 Subject: [PATCH 06/15] bump cargo to fix spurious failure --- src/tools/cargo | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/tools/cargo b/src/tools/cargo index 7f08ace4f1305..f01b232bc7f4d 160000 --- a/src/tools/cargo +++ b/src/tools/cargo @@ -1 +1 @@ -Subproject commit 7f08ace4f1305de7f3b1b0e2f765911957226bd4 +Subproject commit f01b232bc7f4d94f0c4603930a5a96277715eb8c From 4f0ad1c92ca08da6e8dc17838070975762f59714 Mon Sep 17 00:00:00 2001 From: Chris Denton Date: Thu, 6 Jan 2022 02:47:36 +0000 Subject: [PATCH 07/15] Fix CVE-2022-21658 for Windows --- library/std/src/sys/windows/c.rs | 124 +++++++++++- library/std/src/sys/windows/fs.rs | 322 ++++++++++++++++++++++++++++-- 2 files changed, 419 insertions(+), 27 deletions(-) diff --git a/library/std/src/sys/windows/c.rs b/library/std/src/sys/windows/c.rs index 50c4547de85f6..1ef6addc11baa 100644 --- a/library/std/src/sys/windows/c.rs +++ b/library/std/src/sys/windows/c.rs @@ -4,6 +4,7 @@ #![cfg_attr(test, allow(dead_code))] #![unstable(issue = "none", feature = "windows_c")] +use crate::mem; use crate::os::raw::NonZero_c_ulong; use crate::os::raw::{c_char, c_int, c_long, c_longlong, c_uint, c_ulong, c_ushort}; use crate::ptr; @@ -36,6 +37,7 @@ pub type USHORT = c_ushort; pub type SIZE_T = usize; pub type WORD = u16; pub type CHAR = c_char; +pub type CCHAR = c_char; pub type ULONG_PTR = usize; pub type ULONG = c_ulong; pub type NTSTATUS = LONG; @@ -86,16 +88,21 @@ pub const FILE_SHARE_DELETE: DWORD = 0x4; pub const FILE_SHARE_READ: DWORD = 0x1; pub const FILE_SHARE_WRITE: DWORD = 0x2; +pub const FILE_OPEN_REPARSE_POINT: ULONG = 0x200000; +pub const OBJ_DONT_REPARSE: ULONG = 0x1000; + pub const CREATE_ALWAYS: DWORD = 2; pub const CREATE_NEW: DWORD = 1; pub const OPEN_ALWAYS: DWORD = 4; pub const OPEN_EXISTING: DWORD = 3; pub const TRUNCATE_EXISTING: DWORD = 5; +pub const FILE_LIST_DIRECTORY: DWORD = 0x1; pub const FILE_WRITE_DATA: DWORD = 0x00000002; pub const FILE_APPEND_DATA: DWORD = 0x00000004; pub const FILE_WRITE_EA: DWORD = 0x00000010; pub const FILE_WRITE_ATTRIBUTES: DWORD = 0x00000100; +pub const DELETE: DWORD = 0x10000; pub const READ_CONTROL: DWORD = 0x00020000; pub const SYNCHRONIZE: DWORD = 0x00100000; pub const GENERIC_READ: DWORD = 0x80000000; @@ -261,9 +268,61 @@ pub const FD_SETSIZE: usize = 64; pub const STACK_SIZE_PARAM_IS_A_RESERVATION: DWORD = 0x00010000; pub const STATUS_SUCCESS: NTSTATUS = 0x00000000; +pub const STATUS_DELETE_PENDING: NTSTATUS = 0xc0000056_u32 as _; +pub const STATUS_INVALID_PARAMETER: NTSTATUS = 0xc000000d_u32 as _; + +// Equivalent to the `NT_SUCCESS` C preprocessor macro. +// See: https://docs.microsoft.com/en-us/windows-hardware/drivers/kernel/using-ntstatus-values +pub fn nt_success(status: NTSTATUS) -> bool { + status >= 0 +} pub const BCRYPT_USE_SYSTEM_PREFERRED_RNG: DWORD = 0x00000002; +#[repr(C)] +pub struct UNICODE_STRING { + pub Length: u16, + pub MaximumLength: u16, + pub Buffer: *mut u16, +} +impl UNICODE_STRING { + pub fn from_ref(slice: &[u16]) -> Self { + let len = slice.len() * mem::size_of::(); + Self { Length: len as _, MaximumLength: len as _, Buffer: slice.as_ptr() as _ } + } +} +#[repr(C)] +pub struct OBJECT_ATTRIBUTES { + pub Length: ULONG, + pub RootDirectory: HANDLE, + pub ObjectName: *const UNICODE_STRING, + pub Attributes: ULONG, + pub SecurityDescriptor: *mut c_void, + pub SecurityQualityOfService: *mut c_void, +} +impl Default for OBJECT_ATTRIBUTES { + fn default() -> Self { + Self { + Length: mem::size_of::() as _, + RootDirectory: ptr::null_mut(), + ObjectName: ptr::null_mut(), + Attributes: 0, + SecurityDescriptor: ptr::null_mut(), + SecurityQualityOfService: ptr::null_mut(), + } + } +} +#[repr(C)] +pub struct IO_STATUS_BLOCK { + pub Pointer: *mut c_void, + pub Information: usize, +} +impl Default for IO_STATUS_BLOCK { + fn default() -> Self { + Self { Pointer: ptr::null_mut(), Information: 0 } + } +} + #[repr(C)] #[cfg(not(target_pointer_width = "64"))] pub struct WSADATA { @@ -353,9 +412,43 @@ pub enum FILE_INFO_BY_HANDLE_CLASS { FileIdInfo = 18, // 0x12 FileIdExtdDirectoryInfo = 19, // 0x13 FileIdExtdDirectoryRestartInfo = 20, // 0x14 + FileDispositionInfoEx = 21, // 0x15, Windows 10 version 1607 MaximumFileInfoByHandlesClass, } +#[repr(C)] +pub struct FILE_DISPOSITION_INFO { + pub DeleteFile: BOOLEAN, +} + +pub const FILE_DISPOSITION_DELETE: DWORD = 0x1; +pub const FILE_DISPOSITION_POSIX_SEMANTICS: DWORD = 0x2; +pub const FILE_DISPOSITION_IGNORE_READONLY_ATTRIBUTE: DWORD = 0x10; + +#[repr(C)] +pub struct FILE_DISPOSITION_INFO_EX { + pub Flags: DWORD, +} + +#[repr(C)] +#[derive(Default)] +pub struct FILE_ID_BOTH_DIR_INFO { + pub NextEntryOffset: DWORD, + pub FileIndex: DWORD, + pub CreationTime: LARGE_INTEGER, + pub LastAccessTime: LARGE_INTEGER, + pub LastWriteTime: LARGE_INTEGER, + pub ChangeTime: LARGE_INTEGER, + pub EndOfFile: LARGE_INTEGER, + pub AllocationSize: LARGE_INTEGER, + pub FileAttributes: DWORD, + pub FileNameLength: DWORD, + pub EaSize: DWORD, + pub ShortNameLength: CCHAR, + pub ShortName: [WCHAR; 12], + pub FileId: LARGE_INTEGER, + pub FileName: [WCHAR; 1], +} #[repr(C)] pub struct FILE_BASIC_INFO { pub CreationTime: LARGE_INTEGER, @@ -750,16 +843,6 @@ if #[cfg(target_vendor = "uwp")] { pub DeletePending: BOOLEAN, pub Directory: BOOLEAN, } - - #[link(name = "kernel32")] - extern "system" { - pub fn GetFileInformationByHandleEx( - hFile: HANDLE, - fileInfoClass: FILE_INFO_BY_HANDLE_CLASS, - lpFileInformation: LPVOID, - dwBufferSize: DWORD, - ) -> BOOL; - } } } @@ -949,6 +1032,12 @@ extern "system" { cchFilePath: DWORD, dwFlags: DWORD, ) -> DWORD; + pub fn GetFileInformationByHandleEx( + hFile: HANDLE, + fileInfoClass: FILE_INFO_BY_HANDLE_CLASS, + lpFileInformation: LPVOID, + dwBufferSize: DWORD, + ) -> BOOL; pub fn SetFileInformationByHandle( hFile: HANDLE, FileInformationClass: FILE_INFO_BY_HANDLE_CLASS, @@ -1133,6 +1222,21 @@ compat_fn! { compat_fn! { "ntdll": + pub fn NtOpenFile( + FileHandle: *mut HANDLE, + DesiredAccess: ACCESS_MASK, + ObjectAttributes: *const OBJECT_ATTRIBUTES, + IoStatusBlock: *mut IO_STATUS_BLOCK, + ShareAccess: ULONG, + OpenOptions: ULONG + ) -> NTSTATUS { + panic!("`NtOpenFile` not available"); + } + pub fn RtlNtStatusToDosError( + Status: NTSTATUS + ) -> ULONG { + panic!("`RtlNtStatusToDosError` not available"); + } pub fn NtCreateKeyedEvent( KeyedEventHandle: LPHANDLE, DesiredAccess: ACCESS_MASK, diff --git a/library/std/src/sys/windows/fs.rs b/library/std/src/sys/windows/fs.rs index 9859000c8d417..5894c6803498b 100644 --- a/library/std/src/sys/windows/fs.rs +++ b/library/std/src/sys/windows/fs.rs @@ -543,6 +543,218 @@ impl File { })?; Ok(()) } + /// Get only basic file information such as attributes and file times. + fn basic_info(&self) -> io::Result { + unsafe { + let mut info: c::FILE_BASIC_INFO = mem::zeroed(); + let size = mem::size_of_val(&info); + cvt(c::GetFileInformationByHandleEx( + self.handle.as_raw_handle(), + c::FileBasicInfo, + &mut info as *mut _ as *mut libc::c_void, + size as c::DWORD, + ))?; + Ok(info) + } + } + /// Delete using POSIX semantics. + /// + /// Files will be deleted as soon as the handle is closed. This is supported + /// for Windows 10 1607 (aka RS1) and later. However some filesystem + /// drivers will not support it even then, e.g. FAT32. + /// + /// If the operation is not supported for this filesystem or OS version + /// then errors will be `ERROR_NOT_SUPPORTED` or `ERROR_INVALID_PARAMETER`. + fn posix_delete(&self) -> io::Result<()> { + let mut info = c::FILE_DISPOSITION_INFO_EX { + Flags: c::FILE_DISPOSITION_DELETE + | c::FILE_DISPOSITION_POSIX_SEMANTICS + | c::FILE_DISPOSITION_IGNORE_READONLY_ATTRIBUTE, + }; + let size = mem::size_of_val(&info); + cvt(unsafe { + c::SetFileInformationByHandle( + self.handle.as_raw_handle(), + c::FileDispositionInfoEx, + &mut info as *mut _ as *mut _, + size as c::DWORD, + ) + })?; + Ok(()) + } + + /// Delete a file using win32 semantics. The file won't actually be deleted + /// until all file handles are closed. However, marking a file for deletion + /// will prevent anyone from opening a new handle to the file. + fn win32_delete(&self) -> io::Result<()> { + let mut info = c::FILE_DISPOSITION_INFO { DeleteFile: c::TRUE as _ }; + let size = mem::size_of_val(&info); + cvt(unsafe { + c::SetFileInformationByHandle( + self.handle.as_raw_handle(), + c::FileDispositionInfo, + &mut info as *mut _ as *mut _, + size as c::DWORD, + ) + })?; + Ok(()) + } + + /// Fill the given buffer with as many directory entries as will fit. + /// This will remember its position and continue from the last call unless + /// `restart` is set to `true`. + /// + /// The returned bool indicates if there are more entries or not. + /// It is an error if `self` is not a directory. + /// + /// # Symlinks and other reparse points + /// + /// On Windows a file is either a directory or a non-directory. + /// A symlink directory is simply an empty directory with some "reparse" metadata attached. + /// So if you open a link (not its target) and iterate the directory, + /// you will always iterate an empty directory regardless of the target. + fn fill_dir_buff(&self, buffer: &mut DirBuff, restart: bool) -> io::Result { + let class = + if restart { c::FileIdBothDirectoryRestartInfo } else { c::FileIdBothDirectoryInfo }; + + unsafe { + let result = cvt(c::GetFileInformationByHandleEx( + self.handle.as_raw_handle(), + class, + buffer.as_mut_ptr().cast(), + buffer.capacity() as _, + )); + match result { + Ok(_) => Ok(true), + Err(e) if e.raw_os_error() == Some(c::ERROR_NO_MORE_FILES as _) => Ok(false), + Err(e) => Err(e), + } + } + } +} + +/// A buffer for holding directory entries. +struct DirBuff { + buffer: Vec, +} +impl DirBuff { + fn new() -> Self { + const BUFFER_SIZE: usize = 1024; + Self { buffer: vec![0_u8; BUFFER_SIZE] } + } + fn capacity(&self) -> usize { + self.buffer.len() + } + fn as_mut_ptr(&mut self) -> *mut u8 { + self.buffer.as_mut_ptr().cast() + } + /// Returns a `DirBuffIter`. + fn iter(&self) -> DirBuffIter<'_> { + DirBuffIter::new(self) + } +} +impl AsRef<[u8]> for DirBuff { + fn as_ref(&self) -> &[u8] { + &self.buffer + } +} + +/// An iterator over entries stored in a `DirBuff`. +/// +/// Currently only returns file names (UTF-16 encoded). +struct DirBuffIter<'a> { + buffer: Option<&'a [u8]>, + cursor: usize, +} +impl<'a> DirBuffIter<'a> { + fn new(buffer: &'a DirBuff) -> Self { + Self { buffer: Some(buffer.as_ref()), cursor: 0 } + } +} +impl<'a> Iterator for DirBuffIter<'a> { + type Item = &'a [u16]; + fn next(&mut self) -> Option { + use crate::mem::size_of; + let buffer = &self.buffer?[self.cursor..]; + + // Get the name and next entry from the buffer. + // SAFETY: The buffer contains a `FILE_ID_BOTH_DIR_INFO` struct but the + // last field (the file name) is unsized. So an offset has to be + // used to get the file name slice. + let (name, next_entry) = unsafe { + let info = buffer.as_ptr().cast::(); + let next_entry = (*info).NextEntryOffset as usize; + let name = crate::slice::from_raw_parts( + (*info).FileName.as_ptr().cast::(), + (*info).FileNameLength as usize / size_of::(), + ); + (name, next_entry) + }; + + if next_entry == 0 { + self.buffer = None + } else { + self.cursor += next_entry + } + + // Skip `.` and `..` pseudo entries. + const DOT: u16 = b'.' as u16; + match name { + [DOT] | [DOT, DOT] => self.next(), + _ => Some(name), + } + } +} + +/// Open a link relative to the parent directory, ensure no symlinks are followed. +fn open_link_no_reparse(parent: &File, name: &[u16], access: u32) -> io::Result { + // This is implemented using the lower level `NtOpenFile` function as + // unfortunately opening a file relative to a parent is not supported by + // win32 functions. It is however a fundamental feature of the NT kernel. + // + // See https://docs.microsoft.com/en-us/windows/win32/api/winternl/nf-winternl-ntopenfile + unsafe { + let mut handle = ptr::null_mut(); + let mut io_status = c::IO_STATUS_BLOCK::default(); + let name_str = c::UNICODE_STRING::from_ref(name); + use crate::sync::atomic::{AtomicU32, Ordering}; + // The `OBJ_DONT_REPARSE` attribute ensures that we haven't been + // tricked into following a symlink. However, it may not be available in + // earlier versions of Windows. + static ATTRIBUTES: AtomicU32 = AtomicU32::new(c::OBJ_DONT_REPARSE); + let object = c::OBJECT_ATTRIBUTES { + ObjectName: &name_str, + RootDirectory: parent.as_raw_handle(), + Attributes: ATTRIBUTES.load(Ordering::Relaxed), + ..c::OBJECT_ATTRIBUTES::default() + }; + let status = c::NtOpenFile( + &mut handle, + access, + &object, + &mut io_status, + c::FILE_SHARE_DELETE | c::FILE_SHARE_READ | c::FILE_SHARE_WRITE, + // If `name` is a symlink then open the link rather than the target. + c::FILE_OPEN_REPARSE_POINT, + ); + // Convert an NTSTATUS to the more familiar Win32 error codes (aka "DosError") + if c::nt_success(status) { + Ok(File::from_raw_handle(handle)) + } else if status == c::STATUS_DELETE_PENDING { + // We make a special exception for `STATUS_DELETE_PENDING` because + // otherwise this will be mapped to `ERROR_ACCESS_DENIED` which is + // very unhelpful. + Err(io::Error::from_raw_os_error(c::ERROR_DELETE_PENDING as _)) + } else if status == c::STATUS_INVALID_PARAMETER + && ATTRIBUTES.load(Ordering::Relaxed) == c::OBJ_DONT_REPARSE + { + // Try without `OBJ_DONT_REPARSE`. See above. + ATTRIBUTES.store(0, Ordering::Relaxed); + open_link_no_reparse(parent, name, access) + } else { + Err(io::Error::from_raw_os_error(c::RtlNtStatusToDosError(status) as _)) + } + } } impl AsInner for File { @@ -752,30 +964,106 @@ pub fn rmdir(p: &Path) -> io::Result<()> { Ok(()) } +/// Open a file or directory without following symlinks. +fn open_link(path: &Path, access_mode: u32) -> io::Result { + let mut opts = OpenOptions::new(); + opts.access_mode(access_mode); + // `FILE_FLAG_BACKUP_SEMANTICS` allows opening directories. + // `FILE_FLAG_OPEN_REPARSE_POINT` opens a link instead of its target. + opts.custom_flags(c::FILE_FLAG_BACKUP_SEMANTICS | c::FILE_FLAG_OPEN_REPARSE_POINT); + File::open(path, &opts) +} + pub fn remove_dir_all(path: &Path) -> io::Result<()> { - let filetype = lstat(path)?.file_type(); - if filetype.is_symlink() { - // On Windows symlinks to files and directories are removed differently. - // rmdir only deletes dir symlinks and junctions, not file symlinks. - rmdir(path) + let file = open_link(path, c::DELETE | c::FILE_LIST_DIRECTORY)?; + + // Test if the file is not a directory or a symlink to a directory. + if (file.basic_info()?.FileAttributes & c::FILE_ATTRIBUTE_DIRECTORY) == 0 { + return Err(io::Error::from_raw_os_error(c::ERROR_DIRECTORY as _)); + } + let mut delete: fn(&File) -> io::Result<()> = File::posix_delete; + let result = match delete(&file) { + Err(e) if e.kind() == io::ErrorKind::DirectoryNotEmpty => { + match remove_dir_all_recursive(&file, delete) { + // Return unexpected errors. + Err(e) if e.kind() != io::ErrorKind::DirectoryNotEmpty => return Err(e), + result => result, + } + } + // If POSIX delete is not supported for this filesystem then fallback to win32 delete. + Err(e) + if e.raw_os_error() == Some(c::ERROR_NOT_SUPPORTED as i32) + || e.raw_os_error() == Some(c::ERROR_INVALID_PARAMETER as i32) => + { + delete = File::win32_delete; + Err(e) + } + result => result, + }; + if result.is_ok() { + Ok(()) } else { - remove_dir_all_recursive(path) + // This is a fallback to make sure the directory is actually deleted. + // Otherwise this function is prone to failing with `DirectoryNotEmpty` + // due to possible delays between marking a file for deletion and the + // file actually being deleted from the filesystem. + // + // So we retry a few times before giving up. + for _ in 0..5 { + match remove_dir_all_recursive(&file, delete) { + Err(e) if e.kind() == io::ErrorKind::DirectoryNotEmpty => {} + result => return result, + } + } + // Try one last time. + delete(&file) } } -fn remove_dir_all_recursive(path: &Path) -> io::Result<()> { - for child in readdir(path)? { - let child = child?; - let child_type = child.file_type()?; - if child_type.is_dir() { - remove_dir_all_recursive(&child.path())?; - } else if child_type.is_symlink_dir() { - rmdir(&child.path())?; - } else { - unlink(&child.path())?; +fn remove_dir_all_recursive(f: &File, delete: fn(&File) -> io::Result<()>) -> io::Result<()> { + let mut buffer = DirBuff::new(); + let mut restart = true; + // Fill the buffer and iterate the entries. + while f.fill_dir_buff(&mut buffer, restart)? { + for name in buffer.iter() { + // Open the file without following symlinks and try deleting it. + // We try opening will all needed permissions and if that is denied + // fallback to opening without `FILE_LIST_DIRECTORY` permission. + // Note `SYNCHRONIZE` permission is needed for synchronous access. + let mut result = + open_link_no_reparse(&f, name, c::SYNCHRONIZE | c::DELETE | c::FILE_LIST_DIRECTORY); + if matches!(&result, Err(e) if e.kind() == io::ErrorKind::PermissionDenied) { + result = open_link_no_reparse(&f, name, c::SYNCHRONIZE | c::DELETE); + } + match result { + Ok(file) => match delete(&file) { + Err(e) if e.kind() == io::ErrorKind::DirectoryNotEmpty => { + // Iterate the directory's files. + // Ignore `DirectoryNotEmpty` errors here. They will be + // caught when `remove_dir_all` tries to delete the top + // level directory. It can then decide if to retry or not. + match remove_dir_all_recursive(&file, delete) { + Err(e) if e.kind() == io::ErrorKind::DirectoryNotEmpty => {} + result => result?, + } + } + result => result?, + }, + // Ignore error if a delete is already in progress or the file + // has already been deleted. It also ignores sharing violations + // (where a file is locked by another process) as these are + // usually temporary. + Err(e) + if e.raw_os_error() == Some(c::ERROR_DELETE_PENDING as _) + || e.kind() == io::ErrorKind::NotFound + || e.raw_os_error() == Some(c::ERROR_SHARING_VIOLATION as _) => {} + Err(e) => return Err(e), + } } + // Continue reading directory entries without restarting from the beginning, + restart = false; } - rmdir(path) + delete(&f) } pub fn readlink(path: &Path) -> io::Result { From 32ed6e599bb4722efefd78bbc9cd7ec4613cb946 Mon Sep 17 00:00:00 2001 From: Hans Kratz Date: Mon, 17 Jan 2022 09:45:46 +0100 Subject: [PATCH 08/15] Fix CVE-2022-21658 for UNIX-like --- library/std/src/fs/tests.rs | 70 ++++++++ library/std/src/sys/unix/fs.rs | 279 +++++++++++++++++++++++++++++-- library/std/src/sys/unix/weak.rs | 5 +- 3 files changed, 342 insertions(+), 12 deletions(-) diff --git a/library/std/src/fs/tests.rs b/library/std/src/fs/tests.rs index 9a8f1e44f1f1c..4d1959258beb0 100644 --- a/library/std/src/fs/tests.rs +++ b/library/std/src/fs/tests.rs @@ -4,8 +4,10 @@ use crate::fs::{self, File, OpenOptions}; use crate::io::{ErrorKind, SeekFrom}; use crate::path::Path; use crate::str; +use crate::sync::Arc; use crate::sys_common::io::test::{tmpdir, TempDir}; use crate::thread; +use crate::time::{Duration, Instant}; use rand::{rngs::StdRng, RngCore, SeedableRng}; @@ -602,6 +604,21 @@ fn recursive_rmdir_of_symlink() { assert!(canary.exists()); } +#[test] +fn recursive_rmdir_of_file_fails() { + // test we do not delete a directly specified file. + let tmpdir = tmpdir(); + let canary = tmpdir.join("do_not_delete"); + check!(check!(File::create(&canary)).write(b"foo")); + let result = fs::remove_dir_all(&canary); + #[cfg(unix)] + error!(result, "Not a directory"); + #[cfg(windows)] + error!(result, 267); // ERROR_DIRECTORY - The directory name is invalid. + assert!(result.is_err()); + assert!(canary.exists()); +} + #[test] // only Windows makes a distinction between file and directory symlinks. #[cfg(windows)] @@ -621,6 +638,59 @@ fn recursive_rmdir_of_file_symlink() { } } +#[test] +#[ignore] // takes too much time +fn recursive_rmdir_toctou() { + // Test for time-of-check to time-of-use issues. + // + // Scenario: + // The attacker wants to get directory contents deleted, to which he does not have access. + // He has a way to get a privileged Rust binary call `std::fs::remove_dir_all()` on a + // directory he controls, e.g. in his home directory. + // + // The POC sets up the `attack_dest/attack_file` which the attacker wants to have deleted. + // The attacker repeatedly creates a directory and replaces it with a symlink from + // `victim_del` to `attack_dest` while the victim code calls `std::fs::remove_dir_all()` + // on `victim_del`. After a few seconds the attack has succeeded and + // `attack_dest/attack_file` is deleted. + let tmpdir = tmpdir(); + let victim_del_path = tmpdir.join("victim_del"); + let victim_del_path_clone = victim_del_path.clone(); + + // setup dest + let attack_dest_dir = tmpdir.join("attack_dest"); + let attack_dest_dir = attack_dest_dir.as_path(); + fs::create_dir(attack_dest_dir).unwrap(); + let attack_dest_file = tmpdir.join("attack_dest/attack_file"); + File::create(&attack_dest_file).unwrap(); + + let drop_canary_arc = Arc::new(()); + let drop_canary_weak = Arc::downgrade(&drop_canary_arc); + + eprintln!("x: {:?}", &victim_del_path); + + // victim just continuously removes `victim_del` + thread::spawn(move || { + while drop_canary_weak.upgrade().is_some() { + let _ = fs::remove_dir_all(&victim_del_path_clone); + } + }); + + // attacker (could of course be in a separate process) + let start_time = Instant::now(); + while Instant::now().duration_since(start_time) < Duration::from_secs(1000) { + if !attack_dest_file.exists() { + panic!( + "Victim deleted symlinked file outside of victim_del. Attack succeeded in {:?}.", + Instant::now().duration_since(start_time) + ); + } + let _ = fs::create_dir(&victim_del_path); + let _ = fs::remove_dir(&victim_del_path); + let _ = symlink_dir(attack_dest_dir, &victim_del_path); + } +} + #[test] fn unicode_path_is_dir() { assert!(Path::new(".").is_dir()); diff --git a/library/std/src/sys/unix/fs.rs b/library/std/src/sys/unix/fs.rs index a4fff9b2e6473..a150d1a9aacc1 100644 --- a/library/std/src/sys/unix/fs.rs +++ b/library/std/src/sys/unix/fs.rs @@ -64,7 +64,7 @@ use libc::{ dirent64, fstat64, ftruncate64, lseek64, lstat64, off64_t, open64, readdir64_r, stat64, }; -pub use crate::sys_common::fs::{remove_dir_all, try_exists}; +pub use crate::sys_common::fs::try_exists; pub struct File(FileDesc); @@ -228,7 +228,7 @@ pub struct DirEntry { target_os = "fuchsia", target_os = "redox" ))] - name: Box<[u8]>, + name: CString, } #[derive(Clone, Debug)] @@ -455,8 +455,6 @@ impl Iterator for ReadDir { target_os = "illumos" ))] fn next(&mut self) -> Option> { - use crate::slice; - unsafe { loop { // Although readdir_r(3) would be a correct function to use here because @@ -474,14 +472,10 @@ impl Iterator for ReadDir { }; } - let name = (*entry_ptr).d_name.as_ptr(); - let namelen = libc::strlen(name) as usize; - let ret = DirEntry { entry: *entry_ptr, - name: slice::from_raw_parts(name as *const u8, namelen as usize) - .to_owned() - .into_boxed_slice(), + // d_name is guaranteed to be null-terminated. + name: CStr::from_ptr((*entry_ptr).d_name.as_ptr()).to_owned(), dir: Arc::clone(&self.inner), }; if ret.name_bytes() != b"." && ret.name_bytes() != b".." { @@ -664,7 +658,26 @@ impl DirEntry { target_os = "redox" ))] fn name_bytes(&self) -> &[u8] { - &*self.name + self.name.as_bytes() + } + + #[cfg(not(any( + target_os = "solaris", + target_os = "illumos", + target_os = "fuchsia", + target_os = "redox" + )))] + fn name_cstr(&self) -> &CStr { + unsafe { CStr::from_ptr(self.entry.d_name.as_ptr()) } + } + #[cfg(any( + target_os = "solaris", + target_os = "illumos", + target_os = "fuchsia", + target_os = "redox" + ))] + fn name_cstr(&self) -> &CStr { + &self.name } pub fn file_name_os_str(&self) -> &OsStr { @@ -1439,3 +1452,247 @@ pub fn chroot(dir: &Path) -> io::Result<()> { cvt(unsafe { libc::chroot(dir.as_ptr()) })?; Ok(()) } + +pub use remove_dir_impl::remove_dir_all; + +// Fallback for REDOX +#[cfg(target_os = "redox")] +mod remove_dir_impl { + pub use crate::sys_common::fs::remove_dir_all; +} + +// Dynamically choose implementation Macos x86-64: modern for 10.10+, fallback for older versions +#[cfg(all(target_os = "macos", target_arch = "x86_64"))] +mod remove_dir_impl { + use super::{cstr, lstat, Dir, InnerReadDir, ReadDir}; + use crate::ffi::CStr; + use crate::io; + use crate::os::unix::io::{AsRawFd, FromRawFd, IntoRawFd}; + use crate::os::unix::prelude::{OwnedFd, RawFd}; + use crate::path::{Path, PathBuf}; + use crate::sync::Arc; + use crate::sys::weak::weak; + use crate::sys::{cvt, cvt_r}; + use libc::{c_char, c_int, DIR}; + + pub fn openat_nofollow_dironly(parent_fd: Option, p: &CStr) -> io::Result { + weak!(fn openat(c_int, *const c_char, c_int) -> c_int); + let fd = cvt_r(|| unsafe { + openat.get().unwrap()( + parent_fd.unwrap_or(libc::AT_FDCWD), + p.as_ptr(), + libc::O_CLOEXEC | libc::O_RDONLY | libc::O_NOFOLLOW | libc::O_DIRECTORY, + ) + })?; + Ok(unsafe { OwnedFd::from_raw_fd(fd) }) + } + + fn fdreaddir(dir_fd: OwnedFd) -> io::Result<(ReadDir, RawFd)> { + weak!(fn fdopendir(c_int) -> *mut DIR, "fdopendir$INODE64"); + let ptr = unsafe { fdopendir.get().unwrap()(dir_fd.as_raw_fd()) }; + if ptr.is_null() { + return Err(io::Error::last_os_error()); + } + let dirp = Dir(ptr); + // file descriptor is automatically closed by libc::closedir() now, so give up ownership + let new_parent_fd = dir_fd.into_raw_fd(); + // a valid root is not needed because we do not call any functions involving the full path + // of the DirEntrys. + let dummy_root = PathBuf::new(); + Ok(( + ReadDir { + inner: Arc::new(InnerReadDir { dirp, root: dummy_root }), + end_of_stream: false, + }, + new_parent_fd, + )) + } + + fn remove_dir_all_recursive(parent_fd: Option, p: &Path) -> io::Result<()> { + weak!(fn unlinkat(c_int, *const c_char, c_int) -> c_int); + + let pcstr = cstr(p)?; + + // entry is expected to be a directory, open as such + let fd = openat_nofollow_dironly(parent_fd, &pcstr)?; + + // open the directory passing ownership of the fd + let (dir, fd) = fdreaddir(fd)?; + for child in dir { + let child = child?; + match child.entry.d_type { + libc::DT_DIR => { + remove_dir_all_recursive(Some(fd), Path::new(&child.file_name()))?; + } + libc::DT_UNKNOWN => { + match cvt(unsafe { unlinkat.get().unwrap()(fd, child.name_cstr().as_ptr(), 0) }) + { + // type unknown - try to unlink + Err(err) if err.raw_os_error() == Some(libc::EPERM) => { + // if the file is a directory unlink fails with EPERM + remove_dir_all_recursive(Some(fd), Path::new(&child.file_name()))?; + } + result => { + result?; + } + } + } + _ => { + // not a directory -> unlink + cvt(unsafe { unlinkat.get().unwrap()(fd, child.name_cstr().as_ptr(), 0) })?; + } + } + } + + // unlink the directory after removing its contents + cvt(unsafe { + unlinkat.get().unwrap()( + parent_fd.unwrap_or(libc::AT_FDCWD), + pcstr.as_ptr(), + libc::AT_REMOVEDIR, + ) + })?; + Ok(()) + } + + fn remove_dir_all_modern(p: &Path) -> io::Result<()> { + // We cannot just call remove_dir_all_recursive() here because that would not delete a passed + // symlink. No need to worry about races, because remove_dir_all_recursive() does not recurse + // into symlinks. + let attr = lstat(p)?; + if attr.file_type().is_symlink() { + crate::fs::remove_file(p) + } else { + remove_dir_all_recursive(None, p) + } + } + + pub fn remove_dir_all(p: &Path) -> io::Result<()> { + weak!(fn openat(c_int, *const c_char, c_int) -> c_int); + if openat.get().is_some() { + // openat() is available with macOS 10.10+, just like unlinkat() and fdopendir() + remove_dir_all_modern(p) + } else { + // fall back to classic implementation + crate::sys_common::fs::remove_dir_all(p) + } + } +} + +// Modern implementation using openat(), unlinkat() and fdopendir() +#[cfg(not(any(all(target_os = "macos", target_arch = "x86_64"), target_os = "redox")))] +mod remove_dir_impl { + use super::{cstr, lstat, Dir, InnerReadDir, ReadDir}; + use crate::ffi::CStr; + use crate::io; + use crate::os::unix::io::{AsRawFd, FromRawFd, IntoRawFd}; + use crate::os::unix::prelude::{OwnedFd, RawFd}; + use crate::path::{Path, PathBuf}; + use crate::sync::Arc; + use crate::sys::{cvt, cvt_r}; + use libc::{fdopendir, openat, unlinkat}; + + pub fn openat_nofollow_dironly(parent_fd: Option, p: &CStr) -> io::Result { + let fd = cvt_r(|| unsafe { + openat( + parent_fd.unwrap_or(libc::AT_FDCWD), + p.as_ptr(), + libc::O_CLOEXEC | libc::O_RDONLY | libc::O_NOFOLLOW | libc::O_DIRECTORY, + ) + })?; + Ok(unsafe { OwnedFd::from_raw_fd(fd) }) + } + + fn fdreaddir(dir_fd: OwnedFd) -> io::Result<(ReadDir, RawFd)> { + let ptr = unsafe { fdopendir(dir_fd.as_raw_fd()) }; + if ptr.is_null() { + return Err(io::Error::last_os_error()); + } + let dirp = Dir(ptr); + // file descriptor is automatically closed by libc::closedir() now, so give up ownership + let new_parent_fd = dir_fd.into_raw_fd(); + // a valid root is not needed because we do not call any functions involving the full path + // of the DirEntrys. + let dummy_root = PathBuf::new(); + Ok(( + ReadDir { + inner: Arc::new(InnerReadDir { dirp, root: dummy_root }), + #[cfg(not(any( + target_os = "solaris", + target_os = "illumos", + target_os = "fuchsia", + target_os = "redox", + )))] + end_of_stream: false, + }, + new_parent_fd, + )) + } + + fn remove_dir_all_recursive(parent_fd: Option, p: &Path) -> io::Result<()> { + let pcstr = cstr(p)?; + + // entry is expected to be a directory, open as such + let fd = openat_nofollow_dironly(parent_fd, &pcstr)?; + + // open the directory passing ownership of the fd + let (dir, fd) = fdreaddir(fd)?; + for child in dir { + let child = child?; + let child_is_dir = if cfg!(any( + target_os = "solaris", + target_os = "illumos", + target_os = "haiku", + target_os = "vxworks" + )) { + // no d_type in dirent + None + } else { + match child.entry.d_type { + libc::DT_UNKNOWN => None, + libc::DT_DIR => Some(true), + _ => Some(false), + } + }; + match child_is_dir { + Some(true) => { + remove_dir_all_recursive(Some(fd), Path::new(&child.file_name()))?; + } + Some(false) => { + cvt(unsafe { unlinkat(fd, child.name_cstr().as_ptr(), 0) })?; + } + None => match cvt(unsafe { unlinkat(fd, child.name_cstr().as_ptr(), 0) }) { + // type unknown - try to unlink + Err(err) + if err.raw_os_error() == Some(libc::EISDIR) + || err.raw_os_error() == Some(libc::EPERM) => + { + // if the file is a directory unlink fails with EISDIR on Linux and EPERM everyhwere else + remove_dir_all_recursive(Some(fd), Path::new(&child.file_name()))?; + } + result => { + result?; + } + }, + } + } + + // unlink the directory after removing its contents + cvt(unsafe { + unlinkat(parent_fd.unwrap_or(libc::AT_FDCWD), pcstr.as_ptr(), libc::AT_REMOVEDIR) + })?; + Ok(()) + } + + pub fn remove_dir_all(p: &Path) -> io::Result<()> { + // We cannot just call remove_dir_all_recursive() here because that would not delete a passed + // symlink. No need to worry about races, because remove_dir_all_recursive() does not recurse + // into symlinks. + let attr = lstat(p)?; + if attr.file_type().is_symlink() { + crate::fs::remove_file(p) + } else { + remove_dir_all_recursive(None, p) + } + } +} diff --git a/library/std/src/sys/unix/weak.rs b/library/std/src/sys/unix/weak.rs index ba432ec54941c..83ff78fa7a21b 100644 --- a/library/std/src/sys/unix/weak.rs +++ b/library/std/src/sys/unix/weak.rs @@ -28,9 +28,12 @@ use crate::sync::atomic::{self, AtomicUsize, Ordering}; pub(crate) macro weak { (fn $name:ident($($t:ty),*) -> $ret:ty) => ( + weak!(fn $name($($t),*) -> $ret, stringify!($name)); + ), + (fn $name:ident($($t:ty),*) -> $ret:ty, $sym:expr) => ( #[allow(non_upper_case_globals)] static $name: crate::sys::weak::Weak $ret> = - crate::sys::weak::Weak::new(concat!(stringify!($name), '\0')); + crate::sys::weak::Weak::new(concat!($sym, '\0')); ) } From 406cc071d6cfdfdb678bf3d83d766851de95abaf Mon Sep 17 00:00:00 2001 From: Alex Crichton Date: Tue, 4 Jan 2022 08:44:15 -0800 Subject: [PATCH 09/15] Fix CVE-2022-21658 for WASI --- library/std/src/sys/wasi/fs.rs | 71 ++++++++++++++++++++++++++++++---- 1 file changed, 63 insertions(+), 8 deletions(-) diff --git a/library/std/src/sys/wasi/fs.rs b/library/std/src/sys/wasi/fs.rs index 984dda8dc0b4e..00b9ab3b864d3 100644 --- a/library/std/src/sys/wasi/fs.rs +++ b/library/std/src/sys/wasi/fs.rs @@ -16,7 +16,7 @@ use crate::sys::time::SystemTime; use crate::sys::unsupported; use crate::sys_common::{AsInner, FromInner, IntoInner}; -pub use crate::sys_common::fs::{remove_dir_all, try_exists}; +pub use crate::sys_common::fs::try_exists; pub struct File { fd: WasiFd, @@ -130,6 +130,18 @@ impl FileType { } } +impl ReadDir { + fn new(dir: File, root: PathBuf) -> ReadDir { + ReadDir { + cookie: Some(0), + buf: vec![0; 128], + offset: 0, + cap: 0, + inner: Arc::new(ReadDirInner { dir, root }), + } + } +} + impl fmt::Debug for ReadDir { fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { f.debug_struct("ReadDir").finish_non_exhaustive() @@ -512,13 +524,7 @@ pub fn readdir(p: &Path) -> io::Result { opts.directory(true); opts.read(true); let dir = File::open(p, &opts)?; - Ok(ReadDir { - cookie: Some(0), - buf: vec![0; 128], - offset: 0, - cap: 0, - inner: Arc::new(ReadDirInner { dir, root: p.to_path_buf() }), - }) + Ok(ReadDir::new(dir, p.to_path_buf())) } pub fn unlink(p: &Path) -> io::Result<()> { @@ -712,3 +718,52 @@ pub fn copy(from: &Path, to: &Path) -> io::Result { io::copy(&mut reader, &mut writer) } + +pub fn remove_dir_all(path: &Path) -> io::Result<()> { + let (parent, path) = open_parent(path)?; + remove_dir_all_recursive(&parent, &path) +} + +fn remove_dir_all_recursive(parent: &WasiFd, path: &Path) -> io::Result<()> { + // Open up a file descriptor for the directory itself. Note that we don't + // follow symlinks here and we specifically open directories. + // + // At the root invocation of this function this will correctly handle + // symlinks passed to the top-level `remove_dir_all`. At the recursive + // level this will double-check that after the `readdir` call deduced this + // was a directory it's still a directory by the time we open it up. + // + // If the opened file was actually a symlink then the symlink is deleted, + // not the directory recursively. + let mut opts = OpenOptions::new(); + opts.lookup_flags(0); + opts.directory(true); + opts.read(true); + let fd = open_at(parent, path, &opts)?; + if fd.file_attr()?.file_type().is_symlink() { + return parent.unlink_file(osstr2str(path.as_ref())?); + } + + // this "root" is only used by `DirEntry::path` which we don't use below so + // it's ok for this to be a bogus value + let dummy_root = PathBuf::new(); + + // Iterate over all the entries in this directory, and travel recursively if + // necessary + for entry in ReadDir::new(fd, dummy_root) { + let entry = entry?; + let path = crate::str::from_utf8(&entry.name).map_err(|_| { + io::Error::new_const(io::ErrorKind::Uncategorized, &"invalid utf-8 file name found") + })?; + + if entry.file_type()?.is_dir() { + remove_dir_all_recursive(&entry.inner.dir.fd, path.as_ref())?; + } else { + entry.inner.dir.fd.unlink_file(path)?; + } + } + + // Once all this directory's contents are deleted it should be safe to + // delete the directory tiself. + parent.remove_directory(osstr2str(path.as_ref())?) +} From 42cb17e224e1c5ba6292adeb5b77db5bd19a7e3e Mon Sep 17 00:00:00 2001 From: Pietro Albini Date: Wed, 19 Jan 2022 13:14:24 +0100 Subject: [PATCH 10/15] Update std::fs::remove_dir_all documentation --- library/std/src/fs.rs | 12 ++++++++---- 1 file changed, 8 insertions(+), 4 deletions(-) diff --git a/library/std/src/fs.rs b/library/std/src/fs.rs index a14f1e2ecb2c4..f7f7a9ba4e41c 100644 --- a/library/std/src/fs.rs +++ b/library/std/src/fs.rs @@ -2045,13 +2045,17 @@ pub fn remove_dir>(path: P) -> io::Result<()> { /// /// # Platform-specific behavior /// -/// This function currently corresponds to `opendir`, `lstat`, `rm` and `rmdir` functions on Unix -/// and the `FindFirstFile`, `GetFileAttributesEx`, `DeleteFile`, and `RemoveDirectory` functions -/// on Windows. -/// Note that, this [may change in the future][changes]. +/// This function currently corresponds to `openat`, `fdopendir`, `unlinkat` and `lstat` functions +/// on Unix (except for macOS before version 10.10 and REDOX) and the `CreateFileW`, +/// `GetFileInformationByHandleEx`, `SetFileInformationByHandle`, and `NtOpenFile` functions on +/// Windows. Note that, this [may change in the future][changes]. /// /// [changes]: io#platform-specific-behavior /// +/// On macOS before version 10.10 and REDOX this function is not protected against time-of-check to +/// time-of-use (TOCTOU) race conditions, and should not be used in security-sensitive code on +/// those platforms. All other platforms are protected. +/// /// # Errors /// /// See [`fs::remove_file`] and [`fs::remove_dir`]. From 1e17daf1e0104ef096f70c062dd92e4310cd2966 Mon Sep 17 00:00:00 2001 From: Pietro Albini Date: Wed, 19 Jan 2022 13:22:02 +0100 Subject: [PATCH 11/15] Update release notes to include CVE-2022-21658 --- RELEASES.md | 2 ++ 1 file changed, 2 insertions(+) diff --git a/RELEASES.md b/RELEASES.md index 8c358a2879289..bee6da2e673e7 100644 --- a/RELEASES.md +++ b/RELEASES.md @@ -1,11 +1,13 @@ Version 1.58.1 (2022-01-19) =========================== +* Fix race condition in `std::fs::remove_dir_all` ([CVE-2022-21658]) * [Handle captured arguments in the `useless_format` Clippy lint][clippy/8295] * [Move `non_send_fields_in_send_ty` Clippy lint to nursery][clippy/8075] * [Fix wrong error message displayed when some imports are missing][91254] * [Fix rustfmt not formatting generated files from stdin][92912] +[CVE-2022-21658]: https://www.cve.org/CVERecord?id=CVE-2022-21658] [91254]: https://github.com/rust-lang/rust/pull/91254 [92912]: https://github.com/rust-lang/rust/pull/92912 [clippy/8075]: https://github.com/rust-lang/rust-clippy/pull/8075 From cb88166762d30dfc6f7d57679755e79ba5676f4c Mon Sep 17 00:00:00 2001 From: Hans Kratz Date: Thu, 20 Jan 2022 12:35:16 +0100 Subject: [PATCH 12/15] backport missed illumos fix --- library/std/src/sys/unix/fs.rs | 43 ++++++++++++++++++++-------------- 1 file changed, 26 insertions(+), 17 deletions(-) diff --git a/library/std/src/sys/unix/fs.rs b/library/std/src/sys/unix/fs.rs index a150d1a9aacc1..ca7f132a8b344 100644 --- a/library/std/src/sys/unix/fs.rs +++ b/library/std/src/sys/unix/fs.rs @@ -1582,7 +1582,7 @@ mod remove_dir_impl { // Modern implementation using openat(), unlinkat() and fdopendir() #[cfg(not(any(all(target_os = "macos", target_arch = "x86_64"), target_os = "redox")))] mod remove_dir_impl { - use super::{cstr, lstat, Dir, InnerReadDir, ReadDir}; + use super::{cstr, lstat, Dir, DirEntry, InnerReadDir, ReadDir}; use crate::ffi::CStr; use crate::io; use crate::os::unix::io::{AsRawFd, FromRawFd, IntoRawFd}; @@ -1629,6 +1629,30 @@ mod remove_dir_impl { )) } + #[cfg(any( + target_os = "solaris", + target_os = "illumos", + target_os = "haiku", + target_os = "vxworks" + ))] + fn is_dir(_ent: &DirEntry) -> Option { + None + } + + #[cfg(not(any( + target_os = "solaris", + target_os = "illumos", + target_os = "haiku", + target_os = "vxworks" + )))] + fn is_dir(ent: &DirEntry) -> Option { + match ent.entry.d_type { + libc::DT_UNKNOWN => None, + libc::DT_DIR => Some(true), + _ => Some(false), + } + } + fn remove_dir_all_recursive(parent_fd: Option, p: &Path) -> io::Result<()> { let pcstr = cstr(p)?; @@ -1639,22 +1663,7 @@ mod remove_dir_impl { let (dir, fd) = fdreaddir(fd)?; for child in dir { let child = child?; - let child_is_dir = if cfg!(any( - target_os = "solaris", - target_os = "illumos", - target_os = "haiku", - target_os = "vxworks" - )) { - // no d_type in dirent - None - } else { - match child.entry.d_type { - libc::DT_UNKNOWN => None, - libc::DT_DIR => Some(true), - _ => Some(false), - } - }; - match child_is_dir { + match is_dir(&child) { Some(true) => { remove_dir_all_recursive(Some(fd), Path::new(&child.file_name()))?; } From 2923f3a4659326067b5c9dd0ff6e890e4e0681e3 Mon Sep 17 00:00:00 2001 From: Hans Kratz Date: Thu, 20 Jan 2022 12:43:54 +0100 Subject: [PATCH 13/15] Fuchsia apparently does not have DT_UNKNOWN. --- library/std/src/sys/unix/fs.rs | 11 ++++++++++- 1 file changed, 10 insertions(+), 1 deletion(-) diff --git a/library/std/src/sys/unix/fs.rs b/library/std/src/sys/unix/fs.rs index ca7f132a8b344..249838fd655bb 100644 --- a/library/std/src/sys/unix/fs.rs +++ b/library/std/src/sys/unix/fs.rs @@ -1639,11 +1639,20 @@ mod remove_dir_impl { None } + #[cfg(target_os = "fuchsia")] + fn is_dir(ent: &DirEntry) -> Option { + match ent.entry.d_type { + libc::DT_DIR => Some(true), + _ => Some(false), + } + } + #[cfg(not(any( target_os = "solaris", target_os = "illumos", target_os = "haiku", - target_os = "vxworks" + target_os = "vxworks", + target_os = "fuchsia" )))] fn is_dir(ent: &DirEntry) -> Option { match ent.entry.d_type { From 68976d1ac1718311bf6125e137747f61226638db Mon Sep 17 00:00:00 2001 From: Hans Kratz Date: Thu, 20 Jan 2022 13:32:00 +0100 Subject: [PATCH 14/15] name_cstr() is not needed for Redox. --- library/std/src/sys/unix/fs.rs | 7 +------ 1 file changed, 1 insertion(+), 6 deletions(-) diff --git a/library/std/src/sys/unix/fs.rs b/library/std/src/sys/unix/fs.rs index 249838fd655bb..24c1be329b459 100644 --- a/library/std/src/sys/unix/fs.rs +++ b/library/std/src/sys/unix/fs.rs @@ -670,12 +670,7 @@ impl DirEntry { fn name_cstr(&self) -> &CStr { unsafe { CStr::from_ptr(self.entry.d_name.as_ptr()) } } - #[cfg(any( - target_os = "solaris", - target_os = "illumos", - target_os = "fuchsia", - target_os = "redox" - ))] + #[cfg(any(target_os = "solaris", target_os = "illumos", target_os = "fuchsia"))] fn name_cstr(&self) -> &CStr { &self.name } From 1c63ec48b8cbf553d291a04957d86cfd191fcd78 Mon Sep 17 00:00:00 2001 From: Hans Kratz Date: Thu, 20 Jan 2022 13:32:57 +0100 Subject: [PATCH 15/15] Better fix for Fuchsia --- library/std/src/sys/unix/fs.rs | 11 ++--------- 1 file changed, 2 insertions(+), 9 deletions(-) diff --git a/library/std/src/sys/unix/fs.rs b/library/std/src/sys/unix/fs.rs index 24c1be329b459..111639d92b78b 100644 --- a/library/std/src/sys/unix/fs.rs +++ b/library/std/src/sys/unix/fs.rs @@ -1628,20 +1628,13 @@ mod remove_dir_impl { target_os = "solaris", target_os = "illumos", target_os = "haiku", - target_os = "vxworks" + target_os = "vxworks", + target_os = "fuchsia" ))] fn is_dir(_ent: &DirEntry) -> Option { None } - #[cfg(target_os = "fuchsia")] - fn is_dir(ent: &DirEntry) -> Option { - match ent.entry.d_type { - libc::DT_DIR => Some(true), - _ => Some(false), - } - } - #[cfg(not(any( target_os = "solaris", target_os = "illumos",