Possible Double Free Issue in Mir May Compromise Exception Safety in This Crate by kuzeyardabulut · Pull Request #50 · twitter/pants

kuzeyardabulut · 2023-08-05T13:28:33Z

Hi,
I detect several potential double free bugs were detected in your crate via static analysis. This PR contains fixes.

pants/src/rust/engine/src/lib.rs

Lines 882 to 884 in ebf5716

    
           let scheduler = unsafe { Box::from_raw(scheduler_ptr) }; 
        
           let t = f(&scheduler); 
        
           mem::forget(scheduler);

pants/src/rust/engine/src/lib.rs

Lines 895 to 897 in ebf5716

    
           let session = unsafe { Box::from_raw(session_ptr) }; 
        
           let t = f(&session); 
        
           mem::forget(session);

pants/src/rust/engine/src/lib.rs

Lines 908 to 910 in ebf5716

    
           let mut execution_request = unsafe { Box::from_raw(execution_request_ptr) }; 
        
           let t = f(&mut execution_request); 
        
           mem::forget(execution_request);

pants/src/rust/engine/src/lib.rs

Lines 921 to 923 in ebf5716

    
           let mut tasks = unsafe { Box::from_raw(tasks_ptr) }; 
        
           let t = f(&mut tasks); 
        
           mem::forget(tasks);

These bugs primarily emerge when specific functions unwind, predominantly due to the interplay between Box::from_raw and mem::forget. In Rust MIR (Mid-level Intermediate Representation), inserting code between Box::from_raw and mem::forget can compromise exception safety. This is because when these pieces of code unwind, both the Box that was created and the entity to which the pointer refers will be dropped. This scenario, in effect, results in a "double free" situation.

pants/src/rust/engine/src/externs.rs

Lines 679 to 681 in ebf5716

    
           let cs = unsafe { Vec::from_raw_parts(c_ptr, c_len, c_len) }; 
        
           let output = f(&cs); 
        
           mem::forget(cs);

In the second case we shouldn't use code pieces between Vec::from_raw_parts and mem::forget. Because when these codes unwind, the Vec generated will drop as well as the entity which ptr pointed to. This code block can fix it by using mem::ManuallyDrop instead of mem::forget.

kuzeyardabulut added 2 commits August 5, 2023 16:17

adding ManuallyDrop

b87f8f3

adding ManuallyDrop

01812d4

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Possible Double Free Issue in Mir May Compromise Exception Safety in This Crate#50

Possible Double Free Issue in Mir May Compromise Exception Safety in This Crate#50
kuzeyardabulut wants to merge 2 commits intotwitter:masterfrom
kuzeyardabulut:master

kuzeyardabulut commented Aug 5, 2023

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

	let scheduler = unsafe { Box::from_raw(scheduler_ptr) };
	let t = f(&scheduler);
	mem::forget(scheduler);

	let session = unsafe { Box::from_raw(session_ptr) };
	let t = f(&session);
	mem::forget(session);

	let mut execution_request = unsafe { Box::from_raw(execution_request_ptr) };
	let t = f(&mut execution_request);
	mem::forget(execution_request);

	let mut tasks = unsafe { Box::from_raw(tasks_ptr) };
	let t = f(&mut tasks);
	mem::forget(tasks);

	let cs = unsafe { Vec::from_raw_parts(c_ptr, c_len, c_len) };
	let output = f(&cs);
	mem::forget(cs);

Conversation

kuzeyardabulut commented Aug 5, 2023

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant