Skip to content

std.os.windows: add error.UnrecognizedVolume #18952

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Feb 16, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 8 additions & 0 deletions lib/std/os.zig
Original file line number Diff line number Diff line change
Expand Up @@ -5364,13 +5364,18 @@ pub const RealPathError = error{
/// intercepts file system operations and makes them significantly slower
/// in addition to possibly failing with this error code.
AntivirusInterference,

/// On Windows, the volume does not contain a recognized file system. File
/// system drivers might not be loaded, or the volume may be corrupt.
UnrecognizedVolume,
} || UnexpectedError;

/// Return the canonicalized absolute pathname.
/// Expands all symbolic links and resolves references to `.`, `..`, and
/// extra `/` characters in `pathname`.
/// The return value is a slice of `out_buffer`, but not necessarily from the beginning.
/// See also `realpathZ` and `realpathW`.
/// Calling this function is usually a bug.
pub fn realpath(pathname: []const u8, out_buffer: *[MAX_PATH_BYTES]u8) RealPathError![]u8 {
if (builtin.os.tag == .windows) {
const pathname_w = try windows.sliceToPrefixedFileW(null, pathname);
Expand All @@ -5383,6 +5388,7 @@ pub fn realpath(pathname: []const u8, out_buffer: *[MAX_PATH_BYTES]u8) RealPathE
}

/// Same as `realpath` except `pathname` is null-terminated.
/// Calling this function is usually a bug.
pub fn realpathZ(pathname: [*:0]const u8, out_buffer: *[MAX_PATH_BYTES]u8) RealPathError![]u8 {
if (builtin.os.tag == .windows) {
const pathname_w = try windows.cStrToPrefixedFileW(null, pathname);
Expand Down Expand Up @@ -5431,6 +5437,7 @@ pub fn realpathZ(pathname: [*:0]const u8, out_buffer: *[MAX_PATH_BYTES]u8) RealP
}

/// Same as `realpath` except `pathname` is UTF16LE-encoded.
/// Calling this function is usually a bug.
pub fn realpathW(pathname: []const u16, out_buffer: *[MAX_PATH_BYTES]u8) RealPathError![]u8 {
const w = windows;

Expand Down Expand Up @@ -5479,6 +5486,7 @@ pub fn isGetFdPathSupportedOnTarget(os: std.Target.Os) bool {
/// This function is very host-specific and is not universally supported by all hosts.
/// For example, while it generally works on Linux, macOS, FreeBSD or Windows, it is
/// unsupported on WASI.
/// Calling this function is usually a bug.
pub fn getFdPath(fd: fd_t, out_buffer: *[MAX_PATH_BYTES]u8) RealPathError![]u8 {
if (!comptime isGetFdPathSupportedOnTarget(builtin.os)) {
@compileError("querying for canonical path of a handle is unsupported on this host");
Expand Down
55 changes: 43 additions & 12 deletions lib/std/os/windows.zig
Original file line number Diff line number Diff line change
Expand Up @@ -178,7 +178,13 @@ pub fn CreateEventExW(attributes: ?*SECURITY_ATTRIBUTES, nameW: [*:0]const u16,
}
}

pub const DeviceIoControlError = error{ AccessDenied, Unexpected };
pub const DeviceIoControlError = error{
AccessDenied,
/// The volume does not contain a recognized file system. File system
/// drivers might not be loaded, or the volume may be corrupt.
UnrecognizedVolume,
Unexpected,
};

/// A Zig wrapper around `NtDeviceIoControlFile` and `NtFsControlFile` syscalls.
/// It implements similar behavior to `DeviceIoControl` and is meant to serve
Expand Down Expand Up @@ -234,6 +240,7 @@ pub fn DeviceIoControl(
.ACCESS_DENIED => return error.AccessDenied,
.INVALID_DEVICE_REQUEST => return error.AccessDenied, // Not supported by the underlying filesystem
.INVALID_PARAMETER => unreachable,
.UNRECOGNIZED_VOLUME => return error.UnrecognizedVolume,
else => return unexpectedStatus(rc),
}
}
Expand Down Expand Up @@ -606,6 +613,9 @@ pub const CreateSymbolicLinkError = error{
NoDevice,
NetworkNotFound,
BadPathName,
/// The volume does not contain a recognized file system. File system
/// drivers might not be loaded, or the volume may be corrupt.
UnrecognizedVolume,
Unexpected,
};

Expand Down Expand Up @@ -688,12 +698,12 @@ pub fn CreateSymbolicLink(
const target_is_absolute = std.fs.path.isAbsoluteWindowsWTF16(final_target_path);
const symlink_data = SYMLINK_DATA{
.ReparseTag = IO_REPARSE_TAG_SYMLINK,
.ReparseDataLength = @as(u16, @intCast(buf_len - header_len)),
.ReparseDataLength = @intCast(buf_len - header_len),
.Reserved = 0,
.SubstituteNameOffset = @as(u16, @intCast(final_target_path.len * 2)),
.SubstituteNameLength = @as(u16, @intCast(final_target_path.len * 2)),
.SubstituteNameOffset = @intCast(final_target_path.len * 2),
.SubstituteNameLength = @intCast(final_target_path.len * 2),
.PrintNameOffset = 0,
.PrintNameLength = @as(u16, @intCast(final_target_path.len * 2)),
.PrintNameLength = @intCast(final_target_path.len * 2),
.Flags = if (!target_is_absolute) SYMLINK_FLAG_RELATIVE else 0,
};

Expand Down Expand Up @@ -769,7 +779,8 @@ pub fn ReadLink(dir: ?HANDLE, sub_path_w: []const u16, out_buffer: []u8) ReadLin

var reparse_buf: [MAXIMUM_REPARSE_DATA_BUFFER_SIZE]u8 align(@alignOf(REPARSE_DATA_BUFFER)) = undefined;
_ = DeviceIoControl(result_handle, FSCTL_GET_REPARSE_POINT, null, reparse_buf[0..]) catch |err| switch (err) {
error.AccessDenied => unreachable,
error.AccessDenied => return error.Unexpected,
error.UnrecognizedVolume => return error.Unexpected,
else => |e| return e,
};

Expand Down Expand Up @@ -1084,6 +1095,9 @@ pub const GetFinalPathNameByHandleError = error{
BadPathName,
FileNotFound,
NameTooLong,
/// The volume does not contain a recognized file system. File system
/// drivers might not be loaded, or the volume may be corrupt.
UnrecognizedVolume,
Unexpected,
};

Expand Down Expand Up @@ -1174,16 +1188,16 @@ pub fn GetFinalPathNameByHandle(
};
defer CloseHandle(mgmt_handle);

var input_struct = @as(*MOUNTMGR_MOUNT_POINT, @ptrCast(&input_buf[0]));
var input_struct: *MOUNTMGR_MOUNT_POINT = @ptrCast(&input_buf[0]);
input_struct.DeviceNameOffset = @sizeOf(MOUNTMGR_MOUNT_POINT);
input_struct.DeviceNameLength = @as(USHORT, @intCast(volume_name_u16.len * 2));
input_struct.DeviceNameLength = @intCast(volume_name_u16.len * 2);
@memcpy(input_buf[@sizeOf(MOUNTMGR_MOUNT_POINT)..][0 .. volume_name_u16.len * 2], @as([*]const u8, @ptrCast(volume_name_u16.ptr)));

DeviceIoControl(mgmt_handle, IOCTL_MOUNTMGR_QUERY_POINTS, &input_buf, &output_buf) catch |err| switch (err) {
error.AccessDenied => unreachable,
error.AccessDenied => return error.Unexpected,
else => |e| return e,
};
const mount_points_struct = @as(*const MOUNTMGR_MOUNT_POINTS, @ptrCast(&output_buf[0]));
const mount_points_struct: *const MOUNTMGR_MOUNT_POINTS = @ptrCast(&output_buf[0]);

const mount_points = @as(
[*]const MOUNTMGR_MOUNT_POINT,
Expand Down Expand Up @@ -2203,7 +2217,7 @@ pub fn wToPrefixedFileW(dir: ?HANDLE, path: [:0]const u16) !PathSpace {
.unc_absolute => nt_prefix.len + 2,
else => nt_prefix.len,
};
const buf_len = @as(u32, @intCast(path_space.data.len - path_buf_offset));
const buf_len: u32 = @intCast(path_space.data.len - path_buf_offset);
const path_to_get: [:0]const u16 = path_to_get: {
// If dir is null, then we don't need to bother with GetFinalPathNameByHandle because
// RtlGetFullPathName_U will resolve relative paths against the CWD for us.
Expand All @@ -2221,7 +2235,24 @@ pub fn wToPrefixedFileW(dir: ?HANDLE, path: [:0]const u16) !PathSpace {
// canonicalize it. We do this by getting the path of the `dir`
// and appending the relative path to it.
var dir_path_buf: [PATH_MAX_WIDE:0]u16 = undefined;
const dir_path = try GetFinalPathNameByHandle(dir.?, .{}, &dir_path_buf);
const dir_path = GetFinalPathNameByHandle(dir.?, .{}, &dir_path_buf) catch |err| switch (err) {
// This mapping is not correct; it is actually expected
// that calling GetFinalPathNameByHandle might return
// error.UnrecognizedVolume, and in fact has been observed
// in the wild. The problem is that wToPrefixedFileW was
// never intended to make *any* OS syscall APIs. It's only
// supposed to convert a string to one that is eligible to
// be used in the ntdll syscalls.
//
// To solve this, this function needs to no longer call
// GetFinalPathNameByHandle under any conditions, or the
// calling function needs to get reworked to not need to
// call this function.
//
// This may involve making breaking API changes.
Comment on lines +2239 to +2252
Copy link
Collaborator

@squeek502 squeek502 Feb 16, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For context, the dilemma is:

  • ntdll syscalls cannot handle relative paths with .. components
  • In the best case, we can remove .. components trivially (this is what normalizePath/removeDotDirsSanitized does)
  • However, to be able to make a syscall when given a path like ../../foo, it needs to be resolved into an absolute path and given the NT namespace prefix (\??\)
  • RtlGetFullPathName_U can handle the case when resolving a path like ../../foo against the cwd, but not against an arbitrary directory handle
  • Therefore, we use our own GetFinalPathNameByHandle implementation in the case where we (1) can't trivially remove .. components and (2) need to resolve the path against a non-CWD dir

EDIT: So one possibility here is that there's some way to handle a path like ../../foo without needing to resolve it into an absolute path. The other is that this is a bug in our GetFinalPathNameByHandle implementation rather than wToPrefixedFileW (that is, we're not able to get a path of a handle that we should be able to get a path for).

EDIT#2: Here's where the GetFinalPathNameByHandle call was introduced: #16783 and the bug it fixed: #16779

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

However, to be able to make a syscall when given a path like ../../foo, it needs to be resolved into an absolute path and given the NT namespace prefix (??)

Zig file system API should make this fail for all operating systems. You shouldn't be able to open a parent directory from an open directory handle.

The only exception is the current working directory, which is already an exception for other use cases.

This hack should have never been made, and by not making the hack it would have helped us reach that conclusion earlier.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fwiw, I believe it was me who reported ".. doesn’t work on windows” (#16779), and, for my use-case, it is indeed enough if that only works for cwd

Copy link
Collaborator

@squeek502 squeek502 Feb 16, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Zig file system API should make this fail for all operating systems. You shouldn't be able to open a parent directory from an open directory handle.

This might be a bit complicated due to things like #18452 and #7751. That is, resolving .. before symlinks (the Windows way) would be a break from normal non-Windows behavior, and resolving symlinks before .. would require syscalls. Symlinks in general complicate things, too.

There is a potentially relevant RESOLVE_BENEATH flag on Linux (and O_RESOLVE_BENEATH on FreeBSD), though: https://man7.org/linux/man-pages/man2/openat2.2.html

error.UnrecognizedVolume => return error.Unexpected,
else => |e| return e,
};
if (dir_path.len + 1 + path.len > PATH_MAX_WIDE) {
return error.NameTooLong;
}
Expand Down
1 change: 1 addition & 0 deletions src/link.zig
Original file line number Diff line number Diff line change
Expand Up @@ -543,6 +543,7 @@ pub const File = struct {
UnexpectedTable,
UnexpectedValue,
UnknownFeature,
UnrecognizedVolume,
Unseekable,
UnsupportedCpuArchitecture,
UnsupportedVersion,
Expand Down