Commit cf9735a5e0

Andrew Kelley <andrew@ziglang.org>
2023-10-26 05:23:38
link: Coff, MachO, and Wasm all had the same UAF bug
1 parent 098a07d
Changed files (3)
src/link/Coff.zig
@@ -1737,46 +1737,51 @@ pub fn getDeclVAddr(self: *Coff, decl_index: Module.Decl.Index, reloc_info: link
     return 0;
 }
 
-pub fn lowerAnonDecl(self: *Coff, decl_val: InternPool.Index, decl_align: InternPool.Alignment, src_loc: Module.SrcLoc) !codegen.Result {
-    // This is basically the same as lowerUnnamedConst.
-    // example:
-    // const ty = mod.intern_pool.typeOf(decl_val).toType();
-    // const val = decl_val.toValue();
-    // The symbol name can be something like `__anon_{d}` with `@intFromEnum(decl_val)`.
-    // It doesn't have an owner decl because it's just an unnamed constant that might
-    // be used by more than one function, however, its address is being used so we need
-    // to put it in some location.
-    // ...
+pub fn lowerAnonDecl(
+    self: *Coff,
+    decl_val: InternPool.Index,
+    explicit_alignment: InternPool.Alignment,
+    src_loc: Module.SrcLoc,
+) !codegen.Result {
     const gpa = self.base.allocator;
     const mod = self.base.options.module.?;
     const ty = mod.intern_pool.typeOf(decl_val).toType();
-    const gop = try self.anon_decls.getOrPut(gpa, decl_val);
-    const required_alignment = switch (decl_align) {
+    const decl_alignment = switch (explicit_alignment) {
         .none => ty.abiAlignment(mod),
-        else => decl_align,
+        else => explicit_alignment,
     };
-    if (!gop.found_existing or
-        !required_alignment.check(self.getAtom(gop.value_ptr.*).getSymbol(self).value))
-    {
-        const val = decl_val.toValue();
-        const tv = TypedValue{ .ty = ty, .val = val };
-        const name = try std.fmt.allocPrint(gpa, "__anon_{d}", .{@intFromEnum(decl_val)});
-        defer gpa.free(name);
-        const res = self.lowerConst(name, tv, required_alignment, self.rdata_section_index.?, src_loc) catch |err| switch (err) {
-            else => {
-                // TODO improve error message
-                const em = try Module.ErrorMsg.create(gpa, src_loc, "lowerAnonDecl failed with error: {s}", .{
-                    @errorName(err),
-                });
-                return .{ .fail = em };
-            },
-        };
-        const atom_index = switch (res) {
-            .ok => |atom_index| atom_index,
-            .fail => |em| return .{ .fail = em },
-        };
-        gop.value_ptr.* = atom_index;
-    }
+    if (self.anon_decls.get(decl_val)) |atom_index| {
+        const existing_addr = self.getAtom(atom_index).getSymbol(self).value;
+        if (decl_alignment.check(existing_addr))
+            return .ok;
+    }
+
+    const val = decl_val.toValue();
+    const tv = TypedValue{ .ty = ty, .val = val };
+    var name_buf: [32]u8 = undefined;
+    const name = std.fmt.bufPrint(&name_buf, "__anon_{d}", .{
+        @intFromEnum(decl_val),
+    }) catch unreachable;
+    const res = self.lowerConst(
+        name,
+        tv,
+        decl_alignment,
+        self.rdata_section_index.?,
+        src_loc,
+    ) catch |err| switch (err) {
+        error.OutOfMemory => return error.OutOfMemory,
+        else => |e| return .{ .fail = try Module.ErrorMsg.create(
+            gpa,
+            src_loc,
+            "lowerAnonDecl failed with error: {s}",
+            .{@errorName(e)},
+        ) },
+    };
+    const atom_index = switch (res) {
+        .ok => |atom_index| atom_index,
+        .fail => |em| return .{ .fail = em },
+    };
+    try self.anon_decls.put(gpa, decl_val, atom_index);
     return .ok;
 }
 
src/link/MachO.zig
@@ -2866,46 +2866,51 @@ pub fn getDeclVAddr(self: *MachO, decl_index: Module.Decl.Index, reloc_info: Fil
     return 0;
 }
 
-pub fn lowerAnonDecl(self: *MachO, decl_val: InternPool.Index, decl_align: InternPool.Alignment, src_loc: Module.SrcLoc) !codegen.Result {
-    // This is basically the same as lowerUnnamedConst.
-    // example:
-    // const ty = mod.intern_pool.typeOf(decl_val).toType();
-    // const val = decl_val.toValue();
-    // The symbol name can be something like `__anon_{d}` with `@intFromEnum(decl_val)`.
-    // It doesn't have an owner decl because it's just an unnamed constant that might
-    // be used by more than one function, however, its address is being used so we need
-    // to put it in some location.
-    // ...
+pub fn lowerAnonDecl(
+    self: *MachO,
+    decl_val: InternPool.Index,
+    explicit_alignment: InternPool.Alignment,
+    src_loc: Module.SrcLoc,
+) !codegen.Result {
     const gpa = self.base.allocator;
     const mod = self.base.options.module.?;
     const ty = mod.intern_pool.typeOf(decl_val).toType();
-    const gop = try self.anon_decls.getOrPut(gpa, decl_val);
-    const required_alignment = switch (decl_align) {
+    const decl_alignment = switch (explicit_alignment) {
         .none => ty.abiAlignment(mod),
-        else => decl_align,
+        else => explicit_alignment,
     };
-    if (!gop.found_existing or
-        !required_alignment.check(self.getAtom(gop.value_ptr.*).getSymbol(self).n_value))
-    {
-        const val = decl_val.toValue();
-        const tv = TypedValue{ .ty = ty, .val = val };
-        const name = try std.fmt.allocPrint(gpa, "__anon_{d}", .{@intFromEnum(decl_val)});
-        defer gpa.free(name);
-        const res = self.lowerConst(name, tv, required_alignment, self.data_const_section_index.?, src_loc) catch |err| switch (err) {
-            else => {
-                // TODO improve error message
-                const em = try Module.ErrorMsg.create(gpa, src_loc, "lowerAnonDecl failed with error: {s}", .{
-                    @errorName(err),
-                });
-                return .{ .fail = em };
-            },
-        };
-        const atom_index = switch (res) {
-            .ok => |atom_index| atom_index,
-            .fail => |em| return .{ .fail = em },
-        };
-        gop.value_ptr.* = atom_index;
-    }
+    if (self.anon_decls.get(decl_val)) |atom_index| {
+        const existing_addr = self.getAtom(atom_index).getSymbol(self).n_value;
+        if (decl_alignment.check(existing_addr))
+            return .ok;
+    }
+
+    const val = decl_val.toValue();
+    const tv = TypedValue{ .ty = ty, .val = val };
+    var name_buf: [32]u8 = undefined;
+    const name = std.fmt.bufPrint(&name_buf, "__anon_{d}", .{
+        @intFromEnum(decl_val),
+    }) catch unreachable;
+    const res = self.lowerConst(
+        name,
+        tv,
+        decl_alignment,
+        self.data_const_section_index.?,
+        src_loc,
+    ) catch |err| switch (err) {
+        error.OutOfMemory => return error.OutOfMemory,
+        else => |e| return .{ .fail = try Module.ErrorMsg.create(
+            gpa,
+            src_loc,
+            "unable to lower constant value: {s}",
+            .{@errorName(e)},
+        ) },
+    };
+    const atom_index = switch (res) {
+        .ok => |atom_index| atom_index,
+        .fail => |em| return .{ .fail = em },
+    };
+    try self.anon_decls.put(gpa, decl_val, atom_index);
     return .ok;
 }
 
src/link/Wasm.zig
@@ -1702,27 +1702,34 @@ pub fn getDeclVAddr(
     return target_symbol_index;
 }
 
-pub fn lowerAnonDecl(wasm: *Wasm, decl_val: InternPool.Index, decl_align: Alignment, src_loc: Module.SrcLoc) !codegen.Result {
+pub fn lowerAnonDecl(
+    wasm: *Wasm,
+    decl_val: InternPool.Index,
+    explicit_alignment: Alignment,
+    src_loc: Module.SrcLoc,
+) !codegen.Result {
     const gop = try wasm.anon_decls.getOrPut(wasm.base.allocator, decl_val);
     if (!gop.found_existing) {
         const mod = wasm.base.options.module.?;
         const ty = mod.intern_pool.typeOf(decl_val).toType();
         const tv: TypedValue = .{ .ty = ty, .val = decl_val.toValue() };
-        const name = try std.fmt.allocPrintZ(wasm.base.allocator, "__anon_{d}", .{@intFromEnum(decl_val)});
-        defer wasm.base.allocator.free(name);
+        var name_buf: [32]u8 = undefined;
+        const name = std.fmt.bufPrint(&name_buf, "__anon_{d}", .{
+            @intFromEnum(decl_val),
+        }) catch unreachable;
 
         switch (try wasm.lowerConst(name, tv, src_loc)) {
-            .ok => |atom_index| gop.value_ptr.* = atom_index,
+            .ok => |atom_index| wasm.anon_decls.values()[gop.index] = atom_index,
             .fail => |em| return .{ .fail = em },
         }
     }
 
-    const atom = wasm.getAtomPtr(gop.value_ptr.*);
+    const atom = wasm.getAtomPtr(wasm.anon_decls.values()[gop.index]);
     atom.alignment = switch (atom.alignment) {
-        .none => decl_align,
-        else => switch (decl_align) {
+        .none => explicit_alignment,
+        else => switch (explicit_alignment) {
             .none => atom.alignment,
-            else => atom.alignment.maxStrict(decl_align),
+            else => atom.alignment.maxStrict(explicit_alignment),
         },
     };
     return .ok;