Skip to content
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

memory safety when use-after-free occurs of a local function stack variable #3180

Open
jrmuizel opened this issue Sep 5, 2019 · 7 comments
Labels
enhancement Solving this issue will likely involve adding new logic or components to the codebase. frontend Tokenization, parsing, AstGen, Sema, and Liveness.
Milestone

Comments

@jrmuizel
Copy link

jrmuizel commented Sep 5, 2019

The following program builds and runs without complaint. Is there the intent to detect UAFs or should one expect memory safety similar to C?

const std = @import("std");

const P = struct {
    id: u32,
};

pub fn main() !void {
    const alloc = std.heap.page_allocator;
    const k = try alloc.create(P);
    k.id = 5;
    alloc.destroy(k);
    const m = try alloc.create(P);
    std.debug.print("value: {}\n", .{k.id});
}
@andrewrk andrewrk added this to the 0.6.0 milestone Sep 5, 2019
@andrewrk
Copy link
Member

andrewrk commented Sep 5, 2019

Use-after-free is a research area of the #2301 project. See also #1966. Regardless of safety protections, however, the semantics of memory is that a variable's lifetime ends upon exit of the scope it is declared in. Same as C.

@andrewrk
Copy link
Member

andrewrk commented Sep 5, 2019

Oh, see also https://github.com/andrewrk/zig-general-purpose-allocator/. It's not quite ready to merge into std, but that's going to be the go-to allocator for debug builds at least. It has use-after-free detection that works until all virtual memory is exhausted, at which point addresses have to be recycled.

@gunnarahlberg
Copy link

I just happened to have this bug. It still applies, albeit the source should be updated to compile

    ...
    warn("value: {}\n", .{k.id});
`
> value: 2863311530

andrewrk added a commit that referenced this issue Feb 10, 2020
See #3180 for a more comprehensive plan to catch this problem. More
sophisticated control flow analysis is needed to provide compile errors
for returning local variable addresses from a function.
@andrewrk andrewrk modified the milestones: 0.6.0, 0.7.0 Feb 10, 2020
@andrewrk
Copy link
Member

Use after free is now solved as far as safety is concerned with heap allocations, if you use the std lib page_allocator or GeneralPurposeAllocator. This is true even if you use it to e.g. back an ArenaAllocator.

What's left for this issue is to make escaped stack allocations safe.

To revisit the original test case here:

$ ./zig build-exe test.zig 
$ ./test
Segmentation fault at address 0x7f7aba428000
/home/andy/Downloads/zig/build/test.zig:13:39: 0x22fd51 in main (test)
    std.debug.print("value: {}\n", .{k.id});
                                      ^
/home/andy/Downloads/zig/lib/std/start.zig:257:37: 0x204c4d in std.start.posixCallMainAndExit (test)
            const result = root.main() catch |err| {
                                    ^
/home/andy/Downloads/zig/lib/std/start.zig:128:5: 0x20498f in std.start._start (test)
    @call(.{ .modifier = .never_inline }, posixCallMainAndExit, .{});
    ^
Aborted (core dumped)

Here's similar test case. This one uses GeneralPurposeAllocator and is adversarial by forcing the page to stay mapped:

const std = @import("std");

const P = struct {
    id: u32,
};

pub fn main() !void {
    var gpa = std.heap.GeneralPurposeAllocator(.{}){};
    const alloc = &gpa.allocator;
    const k = try alloc.create(P);
    const l = try alloc.create(P);
    k.id = 5;
    alloc.destroy(k);
    const m = try alloc.create(P);
    std.debug.print("value: {x}\n", .{k.id});
}
$ ./zig build-exe test.zig 
$ ./test
value: aaaaaaaa

Here you can see that the freed memory got set to the undefined bit pattern. If you run it with valgrind it will tell you about the branching on undefined:

$ valgrind ./test
==14833== Memcheck, a memory error detector
==14833== Copyright (C) 2002-2017, and GNU GPL'd, by Julian Seward et al.
==14833== Using Valgrind-3.15.0 and LibVEX; rerun with -h for copyright info
==14833== Command: ./test
==14833== 
value: ==14833== Conditional jump or move depends on uninitialised value(s)
==14833==    at 0x2458FC: std.fmt.formatIntUnsigned.240 (fmt.zig:988)
==14833==    by 0x245581: std.fmt.formatInt.239 (fmt.zig:940)
==14833==    by 0x2454CC: std.fmt.formatIntValue.238 (fmt.zig:559)
==14833==    by 0x242293: std.fmt.formatValue.215 (fmt.zig:513)
==14833==    by 0x242217: std.fmt.formatType.214 (fmt.zig:340)
==14833==    by 0x240C22: std.fmt.format.196 (fmt.zig:219)
==14833==    by 0x23A54A: std.io.writer.Writer(std.fs.file.File,std.os.WriteError,std.fs.file.File.write).print.169 (writer.zig:33)
==14833==    by 0x238772: std.debug.print.167 (debug.zig:67)
==14833==    by 0x23065F: main (test.zig:15)
<snip>

@andrewrk andrewrk added enhancement Solving this issue will likely involve adding new logic or components to the codebase. frontend Tokenization, parsing, AstGen, Sema, and Liveness. labels Oct 9, 2020
@andrewrk andrewrk modified the milestones: 0.7.0, 0.8.0 Oct 9, 2020
@andrewrk andrewrk changed the title Use-after-free not detected memory safety in a use-after-free of a local function stack variable Oct 9, 2020
@andrewrk andrewrk changed the title memory safety in a use-after-free of a local function stack variable memory safety when use-after-free occurs of a local function stack variable Oct 9, 2020
@jumpnbrownweasel
Copy link
Contributor

jumpnbrownweasel commented Mar 21, 2021

@andrewrk I think you made a comment on discord about solving the local address problem by doing escape analysis and then allocating memory on the heap for locals that might have escaped (so it would work like in the other cases of use-after-free). The memory allocation scares me for release_safe mode, since this could have a significant cost.

Another idea is to use escape analysis, generate compiler errors for locals that might escape, and have a way for the programmer to suppress these compiler errors in specific instances, an 'unsafe' keyword or something similar.

While Rust is too complex for my taste, one of the things I think they got right was to provide a clear way to isolate the unsafe code. This approach acknowledges that there will always be unsafe code while isolating it in a way that makes you aware of it as the coder, also making it easier to code review and audit. Maybe this approach could be used for other things as well. For example, the compiler could check that all 'undefined' variables are initialized and generate an error when they can't be verified, with a way for the user to suppress them.

I'm sure this is not a new idea and you're more knowledgeable about this than I am (I know nothing about compiler internals). It is probably difficult as well as expensive at compile time. But I wanted to post my reaction and preference.

@wallabra
Copy link

wallabra commented Jul 6, 2022

What's left for this issue is to make escaped stack allocations safe.

Could that be split into its own issue? The core of this one seems solved, judging by the scrollback, unless I misunderstood something. Plus, it'd encourage less milestone juggling.

It would also help repel some of this notion from the general public of Zig as a not entirely safe language, like sugary C.

@190n
Copy link
Contributor

190n commented Aug 29, 2024

Has setting locals to undefined upon going out of scope been discussed as a safety measure? That would likely be useful to catch bugs, either as a temporary solution before any more complex analysis that can create compiler errors, or as a last resort in case some errors are not able to be detected by the compiler.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement Solving this issue will likely involve adding new logic or components to the codebase. frontend Tokenization, parsing, AstGen, Sema, and Liveness.
Projects
Status: To do
Development

No branches or pull requests

6 participants