From 59e9f529df75ee89ba7c72e6e5a6df8b35556ab9 Mon Sep 17 00:00:00 2001 From: Jari Vetoniemi Date: Thu, 15 Feb 2024 21:06:29 +0900 Subject: [PATCH 1/4] std: do not use inferred errors in dynamic_library The error unions for WindowsDynLib and ElfDynLib do not contain all the possible errors. So user code that relies on DynLib.Error will fail to compile. --- lib/std/dynamic_library.zig | 40 +++++++++++++++++++++++++------------ 1 file changed, 27 insertions(+), 13 deletions(-) diff --git a/lib/std/dynamic_library.zig b/lib/std/dynamic_library.zig index f2a7bdc93b..1f8ee1c903 100644 --- a/lib/std/dynamic_library.zig +++ b/lib/std/dynamic_library.zig @@ -111,10 +111,10 @@ pub const ElfDynLib = struct { ElfStringSectionNotFound, ElfSymSectionNotFound, ElfHashTableNotFound, - }; + } || os.OpenError || os.MMapError; /// Trusts the file. Malicious file will be able to execute arbitrary code. - pub fn open(path: []const u8) !ElfDynLib { + pub fn open(path: []const u8) Error!ElfDynLib { const fd = try os.open(path, .{ .ACCMODE = .RDONLY, .CLOEXEC = true }, 0); defer os.close(fd); @@ -250,7 +250,7 @@ pub const ElfDynLib = struct { } /// Trusts the file. Malicious file will be able to execute arbitrary code. - pub fn openZ(path_c: [*:0]const u8) !ElfDynLib { + pub fn openZ(path_c: [*:0]const u8) Error!ElfDynLib { return open(mem.sliceTo(path_c, 0)); } @@ -314,34 +314,48 @@ fn checkver(def_arg: *elf.Verdef, vsym_arg: i32, vername: []const u8, strings: [ return mem.eql(u8, vername, mem.sliceTo(strings + aux.vda_name, 0)); } +test "ElfDynLib" { + if (builtin.os.tag != .linux) { + return error.SkipZigTest; + } + + _ = ElfDynLib.open("invalid_so.so") catch |err| { + try testing.expect(err == error.FileNotFound); + return; + }; +} + pub const WindowsDynLib = struct { - pub const Error = error{FileNotFound}; + pub const Error = error{ + FileNotFound, + InvalidPath, + } || windows.LoadLibraryError; dll: windows.HMODULE, - pub fn open(path: []const u8) !WindowsDynLib { + pub fn open(path: []const u8) Error!WindowsDynLib { return openEx(path, .none); } - pub fn openEx(path: []const u8, flags: windows.LoadLibraryFlags) !WindowsDynLib { - const path_w = try windows.sliceToPrefixedFileW(null, path); + pub fn openEx(path: []const u8, flags: windows.LoadLibraryFlags) Error!WindowsDynLib { + const path_w = windows.sliceToPrefixedFileW(null, path) catch return error.InvalidPath; return openExW(path_w.span().ptr, flags); } - pub fn openZ(path_c: [*:0]const u8) !WindowsDynLib { + pub fn openZ(path_c: [*:0]const u8) Error!WindowsDynLib { return openExZ(path_c, .none); } - pub fn openExZ(path_c: [*:0]const u8, flags: windows.LoadLibraryFlags) !WindowsDynLib { + pub fn openExZ(path_c: [*:0]const u8, flags: windows.LoadLibraryFlags) Error!WindowsDynLib { const path_w = try windows.cStrToPrefixedFileW(null, path_c); return openExW(path_w.span().ptr, flags); } - pub fn openW(path_w: [*:0]const u16) !WindowsDynLib { + pub fn openW(path_w: [*:0]const u16) Error!WindowsDynLib { return openExW(path_w, .none); } - pub fn openExW(path_w: [*:0]const u16, flags: windows.LoadLibraryFlags) !WindowsDynLib { + pub fn openExW(path_w: [*:0]const u16, flags: windows.LoadLibraryFlags) Error!WindowsDynLib { var offset: usize = 0; if (path_w[0] == '\\' and path_w[1] == '?' and path_w[2] == '?' and path_w[3] == '\\') { // + 4 to skip over the \??\ @@ -372,12 +386,12 @@ pub const DlDynLib = struct { handle: *anyopaque, - pub fn open(path: []const u8) !DlDynLib { + pub fn open(path: []const u8) Error!DlDynLib { const path_c = try os.toPosixPath(path); return openZ(&path_c); } - pub fn openZ(path_c: [*:0]const u8) !DlDynLib { + pub fn openZ(path_c: [*:0]const u8) Error!DlDynLib { return DlDynLib{ .handle = system.dlopen(path_c, system.RTLD.LAZY) orelse { return error.FileNotFound; From 7628785983486c612bcadc9a14bd784b627a5d21 Mon Sep 17 00:00:00 2001 From: Jari Vetoniemi Date: Thu, 15 Feb 2024 23:33:08 +0900 Subject: [PATCH 2/4] std: use expectError in dynamic_library tests --- lib/std/dynamic_library.zig | 10 ++-------- 1 file changed, 2 insertions(+), 8 deletions(-) diff --git a/lib/std/dynamic_library.zig b/lib/std/dynamic_library.zig index 1f8ee1c903..ea54750f9a 100644 --- a/lib/std/dynamic_library.zig +++ b/lib/std/dynamic_library.zig @@ -319,10 +319,7 @@ test "ElfDynLib" { return error.SkipZigTest; } - _ = ElfDynLib.open("invalid_so.so") catch |err| { - try testing.expect(err == error.FileNotFound); - return; - }; + try testing.expectError(error.FileNotFound, ElfDynLib.open("invalid_so.so")); } pub const WindowsDynLib = struct { @@ -423,8 +420,5 @@ test "dynamic_library" { else => return error.SkipZigTest, }; - _ = DynLib.open(libname) catch |err| { - try testing.expect(err == error.FileNotFound); - return; - }; + try testing.expectError(error.FileNotFound, DynLib.open(libname)); } From ec2465542cd7bfd58f7bf72552b6f332af2c03f9 Mon Sep 17 00:00:00 2001 From: Veikka Tuominen Date: Sun, 17 Mar 2024 00:01:28 +0200 Subject: [PATCH 3/4] std: adjust DynLib API The cross-platform functions now use an error set that contains all possible errors on every platform. --- lib/std/c.zig | 1 + lib/std/dynamic_library.zig | 79 +++++++++++++++++++++++++++++-------- 2 files changed, 63 insertions(+), 17 deletions(-) diff --git a/lib/std/c.zig b/lib/std/c.zig index d9062a125d..7f8f46ce4d 100644 --- a/lib/std/c.zig +++ b/lib/std/c.zig @@ -1881,6 +1881,7 @@ pub const FILE = opaque {}; pub extern "c" fn dlopen(path: [*:0]const u8, mode: c_int) ?*anyopaque; pub extern "c" fn dlclose(handle: *anyopaque) c_int; pub extern "c" fn dlsym(handle: ?*anyopaque, symbol: [*:0]const u8) ?*anyopaque; +pub extern "c" fn dlerror() ?[*:0]u8; pub extern "c" fn sync() void; pub extern "c" fn syncfs(fd: c_int) c_int; diff --git a/lib/std/dynamic_library.zig b/lib/std/dynamic_library.zig index ea54750f9a..5a17f38429 100644 --- a/lib/std/dynamic_library.zig +++ b/lib/std/dynamic_library.zig @@ -7,14 +7,41 @@ const elf = std.elf; const windows = std.os.windows; const system = std.os.system; -pub const DynLib = switch (builtin.os.tag) { - .linux => if (!builtin.link_libc or builtin.abi == .musl and builtin.link_mode == .static) - ElfDynLib - else - DlDynLib, - .windows => WindowsDynLib, - .macos, .tvos, .watchos, .ios, .freebsd, .netbsd, .openbsd, .dragonfly, .solaris, .illumos => DlDynLib, - else => void, +/// Cross-platform dynamic library loading and symbol lookup. +/// Platform-specific functionality is available through the `inner` field. +pub const DynLib = struct { + const InnerType = switch (builtin.os.tag) { + .linux => if (!builtin.link_libc or builtin.abi == .musl and builtin.link_mode == .static) + ElfDynLib + else + DlDynLib, + .windows => WindowsDynLib, + .macos, .tvos, .watchos, .ios, .freebsd, .netbsd, .openbsd, .dragonfly, .solaris, .illumos => DlDynLib, + else => @compileError("unsupported platform"), + }; + + inner: InnerType, + + pub const Error = ElfDynLib.Error || DlDynLib.Error || WindowsDynLib.Error; + + /// Trusts the file. Malicious file will be able to execute arbitrary code. + pub fn open(path: []const u8) Error!DynLib { + return .{ .inner = try InnerType.open(path) }; + } + + /// Trusts the file. Malicious file will be able to execute arbitrary code. + pub fn openZ(path_c: [*:0]const u8) Error!DynLib { + return .{ .inner = try InnerType.open(path_c) }; + } + + /// Trusts the file. + pub fn close(self: *DynLib) void { + return self.inner.close(); + } + + pub fn lookup(self: *DynLib, comptime T: type, name: [:0]const u8) ?T { + return self.inner.lookup(T, name); + } }; // The link_map structure is not completely specified beside the fields @@ -59,12 +86,12 @@ pub fn get_DYNAMIC() ?[*]elf.Dyn { return @extern([*]elf.Dyn, .{ .name = "_DYNAMIC", .linkage = .weak }); } -pub fn linkmap_iterator(phdrs: []elf.Phdr) !LinkMap.Iterator { +pub fn linkmap_iterator(phdrs: []elf.Phdr) error{InvalidExe}!LinkMap.Iterator { _ = phdrs; const _DYNAMIC = get_DYNAMIC() orelse { // No PT_DYNAMIC means this is either a statically-linked program or a // badly corrupted dynamically-linked one. - return LinkMap.Iterator{ .current = null }; + return .{ .current = null }; }; const link_map_ptr = init: { @@ -89,10 +116,10 @@ pub fn linkmap_iterator(phdrs: []elf.Phdr) !LinkMap.Iterator { else => {}, } } - return LinkMap.Iterator{ .current = null }; + return .{ .current = null }; }; - return LinkMap.Iterator{ .current = link_map_ptr }; + return .{ .current = link_map_ptr }; } pub const ElfDynLib = struct { @@ -239,7 +266,7 @@ pub const ElfDynLib = struct { } } - return ElfDynLib{ + return .{ .memory = all_loaded_mem, .strings = maybe_strings orelse return error.ElfStringSectionNotFound, .syms = maybe_syms orelse return error.ElfSymSectionNotFound, @@ -260,7 +287,7 @@ pub const ElfDynLib = struct { self.* = undefined; } - pub fn lookup(self: *ElfDynLib, comptime T: type, name: [:0]const u8) ?T { + pub fn lookup(self: *const ElfDynLib, comptime T: type, name: [:0]const u8) ?T { if (self.lookupAddress("", name)) |symbol| { return @as(T, @ptrFromInt(symbol)); } else { @@ -268,6 +295,7 @@ pub const ElfDynLib = struct { } } + /// ElfDynLib specific /// Returns the address of the symbol pub fn lookupAddress(self: *const ElfDynLib, vername: []const u8, name: []const u8) ?usize { const maybe_versym = if (self.verdef == null) null else self.versym; @@ -334,6 +362,8 @@ pub const WindowsDynLib = struct { return openEx(path, .none); } + /// WindowsDynLib specific + /// Opens dynamic library with specified library loading flags. pub fn openEx(path: []const u8, flags: windows.LoadLibraryFlags) Error!WindowsDynLib { const path_w = windows.sliceToPrefixedFileW(null, path) catch return error.InvalidPath; return openExW(path_w.span().ptr, flags); @@ -343,15 +373,20 @@ pub const WindowsDynLib = struct { return openExZ(path_c, .none); } + /// WindowsDynLib specific + /// Opens dynamic library with specified library loading flags. pub fn openExZ(path_c: [*:0]const u8, flags: windows.LoadLibraryFlags) Error!WindowsDynLib { const path_w = try windows.cStrToPrefixedFileW(null, path_c); return openExW(path_w.span().ptr, flags); } + /// WindowsDynLib specific pub fn openW(path_w: [*:0]const u16) Error!WindowsDynLib { return openExW(path_w, .none); } + /// WindowsDynLib specific + /// Opens dynamic library with specified library loading flags. pub fn openExW(path_w: [*:0]const u16, flags: windows.LoadLibraryFlags) Error!WindowsDynLib { var offset: usize = 0; if (path_w[0] == '\\' and path_w[1] == '?' and path_w[2] == '?' and path_w[3] == '\\') { @@ -359,7 +394,7 @@ pub const WindowsDynLib = struct { offset = 4; } - return WindowsDynLib{ + return .{ .dll = try windows.LoadLibraryExW(path_w + offset, flags), }; } @@ -389,7 +424,7 @@ pub const DlDynLib = struct { } pub fn openZ(path_c: [*:0]const u8) Error!DlDynLib { - return DlDynLib{ + return .{ .handle = system.dlopen(path_c, system.RTLD.LAZY) orelse { return error.FileNotFound; }, @@ -397,7 +432,10 @@ pub const DlDynLib = struct { } pub fn close(self: *DlDynLib) void { - _ = system.dlclose(self.handle); + switch (std.os.errno(system.dlclose(self.handle))) { + .SUCCESS => return, + else => unreachable, + } self.* = undefined; } @@ -410,6 +448,13 @@ pub const DlDynLib = struct { return null; } } + + /// DlDynLib specific + /// Returns human readable string describing most recent error than occurred from `lookup` + /// or `null` if no error has occurred since initialization or when `getError` was last called. + pub fn getError() ?[:0]const u8 { + return mem.span(system.dlerror()); + } }; test "dynamic_library" { From e6f74b78ef48bc1d23ccd98d042534f0bbe3eb80 Mon Sep 17 00:00:00 2001 From: Veikka Tuominen Date: Sun, 17 Mar 2024 09:06:54 +0200 Subject: [PATCH 4/4] std: define error set of `toPosixPath` --- lib/std/dynamic_library.zig | 2 +- lib/std/os.zig | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/lib/std/dynamic_library.zig b/lib/std/dynamic_library.zig index 5a17f38429..7a31c39f44 100644 --- a/lib/std/dynamic_library.zig +++ b/lib/std/dynamic_library.zig @@ -414,7 +414,7 @@ pub const WindowsDynLib = struct { }; pub const DlDynLib = struct { - pub const Error = error{FileNotFound}; + pub const Error = error{ FileNotFound, NameTooLong }; handle: *anyopaque, diff --git a/lib/std/os.zig b/lib/std/os.zig index 417e571145..2138f5cd20 100644 --- a/lib/std/os.zig +++ b/lib/std/os.zig @@ -5979,7 +5979,7 @@ pub fn sched_getaffinity(pid: pid_t) SchedGetAffinityError!cpu_set_t { /// Used to convert a slice to a null terminated slice on the stack. /// TODO https://github.com/ziglang/zig/issues/287 -pub fn toPosixPath(file_path: []const u8) ![MAX_PATH_BYTES - 1:0]u8 { +pub fn toPosixPath(file_path: []const u8) error{NameTooLong}![MAX_PATH_BYTES - 1:0]u8 { if (std.debug.runtime_safety) assert(std.mem.indexOfScalar(u8, file_path, 0) == null); var path_with_null: [MAX_PATH_BYTES - 1:0]u8 = undefined; // >= rather than > to make room for the null byte