Comments (28)
It was indeed confirmed, that the error was in the Network Drive SFTP driver.
Details can be found winfsp/winfsp#561
from rust.
If you end up creating an issue at https://github.com/winfsp/sshfs-win/, please link to it. :)
from rust.
BTW: mimicing the remove_dir_all
with this function works:
use std::{fs, io, path::Path};
fn main() {
// fs::remove_dir_all("Z:\\latest\\.xxx").unwrap();
remove_dir_all_alternative(Path::new("Z:\\latest\\.xxx")).unwrap();
}
fn remove_dir_all_alternative(path: &Path) -> io::Result<()> {
for entry in fs::read_dir(path)? {
let path = entry?.path();
if path.is_dir() {
remove_dir_all_alternative(&path)?;
} else {
fs::remove_file(path)?;
}
}
fs::remove_dir(path)?;
Ok(())
}
from rust.
cc @ChrisDenton
from rust.
Does it work if you call remove_dir_all
repeatedly? Maybe with a sleep between calls.
from rust.
When running this
use std::{fs, thread};
fn main() {
let path = "Z:\\latest\\.xxx";
// let path = "C:\\temp\\.xxx";
for i in 1..10 {
match fs::remove_dir_all(path) {
Ok(_) => {
println!("Success!");
break;
}
Err(_) => {
println!("Failed! Retrying... ({}/10)", i);
thread::sleep(std::time::Duration::from_millis(50));
}
}
}
}
I get:
Running `target\debug\remover.exe`
Failed! Retrying... (1/10)
Failed! Retrying... (2/10)
Failed! Retrying... (3/10)
Failed! Retrying... (4/10)
Failed! Retrying... (5/10)
Failed! Retrying... (6/10)
Failed! Retrying... (7/10)
Failed! Retrying... (8/10)
Failed! Retrying... (9/10)
when I run this with the C:\temp\.xxx
path, it works on first try.
Also playing with the sleep didn't help.
from rust.
Does anything get deleted or is it all still there?
from rust.
No, actually nothing gets deleted.
from rust.
BTW: I'm using x86_64-pc-windows-gnu
as target
from rust.
Ok, thank you. Rust's implementation of remove_dir_all
is rather gnarly to defend against CVE-2022-21658. My best guess is the filesystem is either returning ERROR_DIR_NOT_EMPTY
when deleting a file in the way we do or else it's returning success but not deleting the file.
Which sftp client are you using btw?
from rust.
I'm using this driver: https://github.com/winfsp/sshfs-win
from rust.
Hm, a cygwin port of a unix tool? That may make error codes misleading because it's translating them to/from POSIX error codes.
I've extracted the part that I think is failing into a test program you could try. It should either fail when calling the custom delete
function or with the assert!
at the end.
# Cargo.toml
[dependencies]
windows-sys = { version = "0.52.0", features = ["Win32_Foundation", "Win32_Storage_FileSystem"] }
// src/main.rs
use std::fs::File;
use std::os::windows::fs::OpenOptionsExt;
use std::os::windows::io::AsRawHandle;
use windows_sys::Win32::Storage::FileSystem::{
FileDispositionInfoEx, SetFileInformationByHandle, DELETE, FILE_DISPOSITION_FLAG_DELETE,
FILE_DISPOSITION_FLAG_IGNORE_READONLY_ATTRIBUTE, FILE_DISPOSITION_FLAG_POSIX_SEMANTICS,
FILE_DISPOSITION_INFO_EX,
};
fn main() {
let path = r"Z:\latest\.xxx\some-file.txt";
let file = File::options().access_mode(DELETE).open(path).unwrap();
delete(&file).unwrap();
drop(file);
assert!(!std::path::Path::new(path).exists());
}
fn delete(file: &File) -> std::io::Result<()> {
unsafe {
let handle = file.as_raw_handle() as _;
let info = FILE_DISPOSITION_INFO_EX {
Flags: FILE_DISPOSITION_FLAG_DELETE
| FILE_DISPOSITION_FLAG_POSIX_SEMANTICS
| FILE_DISPOSITION_FLAG_IGNORE_READONLY_ATTRIBUTE,
};
let result = SetFileInformationByHandle(
handle,
FileDispositionInfoEx,
std::ptr::from_ref(&info).cast(),
std::mem::size_of::<FILE_DISPOSITION_INFO_EX>() as u32,
);
if result == 0 {
Err(std::io::Error::last_os_error())
} else {
Ok(())
}
}
}
from rust.
:) This works perfectly fine. The file r"Z:\latest\.xxx\some-file.txt"
is gone and the program exited with success.
from rust.
Huh interesting, that unfortunately means the failure is somewhere in the more complicated bit of code.
from rust.
I tried a different SFTP integartion: https://www.nsoftware.com/sftpdrive
When deleting it from network drives with that integration, the remove_dir_all
works like a charm.
from rust.
When I have some free time I'll extract the full remove_dir_all
code into a standalone program so the exact cause of the error can be determined. It's likely either the special way we open files or else directory iteration.
In the meantime you could also try the remove_dir_all
crate. This uses a similar method to std but it's just different enough that it may work. It might also have better error reporting.
from rust.
Is there something like strace for Windows? It would really help here.
from rust.
Thx for all this effort.
I already tried to extract the remove_dir_all_iterative
into my program, but it has too many internal dependencies for my current Rust-skills :)
I'll try to find out if the used SFTP integration has some sort of log. If I find something, I'll add it here.
from rust.
Is there something like strace for Windows? It would really help here.
I'd usually use Process Monitor for this.
from rust.
@guenhter Could you try running the original code, but attached to Process Monitor, and report what call fails and with which error?
from rust.
This tool is awesome:
here a screenshot and the zipped log of the process-monitor.
from rust.
And this happens, If I delete a directory with only a file inside and no more subdirs (successful operation).
from rust.
In the first screenshot we see the source of the problem:
CreateFile
is called with the path .xxx\y\some.txt
(with DesiredAccess: Delete, Synchronize, Disposition: Open, Options: OPen Reparse Point, Attribtes: n/a, ShareMode: Read, Write, Delete, AllocationSize: n/a).
The file itself seems to exist. But the call fails with BAD NETWORK PATH
.
The next step is to figure out whether this is a problem with the Rust code or with https://github.com/winfsp/sshfs-win.
from rust.
When looking at the code, the only thing what is different of the root directory and the subdirectories is the way the file-handles to the directories are opened:
// File handle to root dir
let file = open_link(path, c::DELETE | c::FILE_LIST_DIRECTORY)?;
// File handle to sub directories
let child_dir = open_link_no_reparse(
&dir,
&name,
c::SYNCHRONIZE | c::DELETE | c::FILE_LIST_DIRECTORY,
);
The rest is (file deletion handling, ...) seems idential. If we could extract the imporant parts of open_link_no_reparse
, I guess it would be easy for me to test if this subtle different is the issue...
from rust.
Hmm, that is the code for opening subdirectories based on a parent directory handle, which is the core of the CVE mitigation. It could be there's some combination of flags it doesn't like. If it doesn't support this at all then that's more of a problem and will be difficult to reconcile.
Here's a very cut down program that basically copy/pastes the open calls but doesn't do directory iteration:
Rust program
[dependencies]
windows-sys = { version = "0.52.0", features = [
"Wdk_Foundation",
"Wdk_Storage_FileSystem",
"Win32_Foundation",
"Win32_Storage_FileSystem",
"Win32_System_IO",
"Win32_System_Kernel",
] }
use std::fs::File;
use std::io;
use std::os::windows::fs::OpenOptionsExt;
use std::os::windows::io::AsRawHandle;
use std::os::windows::io::FromRawHandle;
use std::path::Path;
use std::ptr;
use windows_sys::Wdk::Foundation::OBJECT_ATTRIBUTES;
use windows_sys::Wdk::Storage::FileSystem::{NtCreateFile, FILE_OPEN, FILE_OPEN_REPARSE_POINT};
use windows_sys::Win32::Foundation::{
RtlNtStatusToDosError, ERROR_DELETE_PENDING, STATUS_DELETE_PENDING, STATUS_PENDING,
UNICODE_STRING,
};
use windows_sys::Win32::Storage::FileSystem::{
FileDispositionInfoEx, SetFileInformationByHandle, DELETE, FILE_DISPOSITION_FLAG_DELETE,
FILE_DISPOSITION_FLAG_IGNORE_READONLY_ATTRIBUTE, FILE_DISPOSITION_FLAG_POSIX_SEMANTICS,
FILE_DISPOSITION_INFO_EX, FILE_FLAG_BACKUP_SEMANTICS, FILE_FLAG_OPEN_REPARSE_POINT,
FILE_LIST_DIRECTORY, FILE_SHARE_DELETE, FILE_SHARE_READ, FILE_SHARE_WRITE, SYNCHRONIZE,
};
use windows_sys::Win32::System::Kernel::OBJ_DONT_REPARSE;
use windows_sys::Win32::System::IO::{IO_STATUS_BLOCK, IO_STATUS_BLOCK_0};
fn utf16(s: &str) -> Vec<u16> {
s.encode_utf16().collect()
}
fn main() {
let path = r"Z:\latest\.xxx";
let subpath = "y";
let filename = "some.txt";
let dir = open_link(path.as_ref(), DELETE | FILE_LIST_DIRECTORY).unwrap();
let subdir = open_link_no_reparse(
&dir,
&utf16(subpath),
SYNCHRONIZE | DELETE | FILE_LIST_DIRECTORY,
)
.unwrap();
let f = open_link_no_reparse(&subdir, &utf16(filename), SYNCHRONIZE | DELETE).unwrap();
f.posix_delete().unwrap();
}
trait FileExt {
fn posix_delete(&self) -> io::Result<()>;
}
impl FileExt for File {
fn posix_delete(&self) -> io::Result<()> {
unsafe {
let handle = self.as_raw_handle() as _;
let info = FILE_DISPOSITION_INFO_EX {
Flags: FILE_DISPOSITION_FLAG_DELETE
| FILE_DISPOSITION_FLAG_POSIX_SEMANTICS
| FILE_DISPOSITION_FLAG_IGNORE_READONLY_ATTRIBUTE,
};
let result = SetFileInformationByHandle(
handle,
FileDispositionInfoEx,
std::ptr::from_ref(&info).cast(),
std::mem::size_of::<FILE_DISPOSITION_INFO_EX>() as u32,
);
if result == 0 {
Err(std::io::Error::last_os_error())
} else {
Ok(())
}
}
}
}
fn open_link(path: &Path, access_mode: u32) -> io::Result<File> {
File::options()
.access_mode(access_mode)
.custom_flags(FILE_FLAG_BACKUP_SEMANTICS | FILE_FLAG_OPEN_REPARSE_POINT)
.open(path)
}
fn open_link_no_reparse(parent: &File, name: &[u16], access: u32) -> io::Result<File> {
unsafe {
let mut handle = 0;
let mut io_status = IO_STATUS_BLOCK {
Anonymous: IO_STATUS_BLOCK_0 {
Status: STATUS_PENDING,
},
Information: 0,
};
let mut name_str = UNICODE_STRING {
Length: (name.len() * 2) as u16,
MaximumLength: (name.len() * 2) as u16,
Buffer: name.as_ptr().cast_mut(),
};
let object = OBJECT_ATTRIBUTES {
Length: std::mem::size_of::<OBJECT_ATTRIBUTES>() as u32,
ObjectName: &mut name_str,
RootDirectory: parent.as_raw_handle() as _,
Attributes: OBJ_DONT_REPARSE as _,
SecurityDescriptor: ptr::null(),
SecurityQualityOfService: ptr::null(),
};
let status = NtCreateFile(
&mut handle,
access,
&object,
&mut io_status,
ptr::null_mut(),
0,
FILE_SHARE_DELETE | FILE_SHARE_READ | FILE_SHARE_WRITE,
FILE_OPEN,
FILE_OPEN_REPARSE_POINT,
ptr::null_mut(),
0,
);
eprintln!("NtCreateFile: {status:#X}");
if status >= 0 {
Ok(File::from_raw_handle(handle as _))
} else if status == STATUS_DELETE_PENDING {
Err(io::Error::from_raw_os_error(ERROR_DELETE_PENDING as i32))
} else {
Err(io::Error::from_raw_os_error(
RtlNtStatusToDosError(status) as _
))
}
}
}
from rust.
Very nice. This reproduces the issue.
So the combination of
let subdir = open_link_no_reparse(...);
let f = open_link_no_reparse(&subdir, ...);
always fails,
and the combination of
let dir = open_link(...).unwrap();
let f = open_link_no_reparse(&dir, ...);
works.
from rust.
That sounds like an issue with the file system driver to me.
from rust.
Yeah, everything points in that direction.
Digging deeper into the two open_link...
functions will reveal, that it boils down to those two functions:
CreateFileW(...) // for open_link
NtCreateFile(...) // for open_link_no_reparse
I'll isolate those calls even further and then open an issue on the drivers side. I doubt that it has to do anything with the paramters of NtCreateFile
becuase it works for the local filesystem, samba, and even for SFTP with another driver.
Thank you very much guys for the incredible support!
from rust.
Related Issues (20)
- Tracking Issue for `core::clone::CloneToUninit` trait HOT 6
- trying to get pointer to extern typed field causes runtime panic HOT 6
- Tracking Issue for `-Z verbose-asm`
- `unreachable_code` detection and/or lint seems broken HOT 1
- Internal Compiler Error when using extern types HOT 7
- Tracking Issue for consolidating command invocations in bootstrap
- electric bug display with seismic detector HOT 1
- Can `DynMetadata` be made constructable? HOT 2
- ICE: type map: `left == right`
- C-variadic functions may not be extern "C-unwind" HOT 3
- Tracking Issue for `mpmc`
- rustc panicked: [evaluate_obligation] evaluating trait selection obligation HOT 2
- Mistaken type annotation on documentation page for HashMap HOT 3
- ICE: `no def-id for fresh lifetime` HOT 1
- Confusing documentation for wasm32-wasip1-threads HOT 1
- Main thread thread-local destructors are not always run HOT 2
- File format not recognized when linking rmeta file for sparc-linux-unknown-gnu HOT 7
- Unhelpful compiler suggestion when trying to dereference a pointer that is not `Copy` HOT 5
- Infinite recursion when reordering bounds in where clause
- Different behavior when changing `opt-level` HOT 4
Recommend Projects
-
React
A declarative, efficient, and flexible JavaScript library for building user interfaces.
-
Vue.js
🖖 Vue.js is a progressive, incrementally-adoptable JavaScript framework for building UI on the web.
-
Typescript
TypeScript is a superset of JavaScript that compiles to clean JavaScript output.
-
TensorFlow
An Open Source Machine Learning Framework for Everyone
-
Django
The Web framework for perfectionists with deadlines.
-
Laravel
A PHP framework for web artisans
-
D3
Bring data to life with SVG, Canvas and HTML. 📊📈🎉
-
Recommend Topics
-
javascript
JavaScript (JS) is a lightweight interpreted programming language with first-class functions.
-
web
Some thing interesting about web. New door for the world.
-
server
A server is a program made to process requests and deliver data to clients.
-
Machine learning
Machine learning is a way of modeling and interpreting data that allows a piece of software to respond intelligently.
-
Visualization
Some thing interesting about visualization, use data art
-
Game
Some thing interesting about game, make everyone happy.
Recommend Org
-
Facebook
We are working to build community through open source technology. NB: members must have two-factor auth.
-
Microsoft
Open source projects and samples from Microsoft.
-
Google
Google ❤️ Open Source for everyone.
-
Alibaba
Alibaba Open Source for everyone
-
D3
Data-Driven Documents codes.
-
Tencent
China tencent open source team.
from rust.