Skip to content

Commit

Permalink
Fix memory leak on sys_exec
Browse files Browse the repository at this point in the history
  • Loading branch information
rafalmiel committed May 27, 2024
1 parent 288b0a4 commit 64ee581
Show file tree
Hide file tree
Showing 8 changed files with 38 additions and 15 deletions.
10 changes: 8 additions & 2 deletions cykusz-rs/src/arch/x86_64/task/mod.rs
Original file line number Diff line number Diff line change
@@ -1,4 +1,3 @@
use alloc::string::String;
use alloc::vec::Vec;
use core::mem::size_of;
use core::ptr::Unique;
Expand All @@ -19,6 +18,7 @@ use crate::arch::raw::segmentation::SegmentSelector;
use crate::arch::syscall::SyscallFrame;
use crate::arch::utils::StackHelper;
use crate::drivers::elf::ElfHeader;
use crate::kernel::fs::dirent::DirEntryItem;
use crate::kernel::mm::virt::PageFlags;
use crate::kernel::mm::{allocate, VirtAddr, PAGE_SIZE};
use crate::kernel::mm::{Frame, PhysAddr};
Expand Down Expand Up @@ -100,6 +100,7 @@ fn prepare_p4<'a>() -> &'a mut P4Table {

#[allow(unused)]
fn prepare_tls(vm: &VM, p_table: &mut P4Table, tls: &TlsVmInfo) -> VirtAddr {
logln!("prepare tls");
let mmap = vm
.mmap_vm(
Some(tls.mmap_addr_hint),
Expand Down Expand Up @@ -450,12 +451,14 @@ impl Task {
hdr: &ElfHeader,
vm: &VM,
tls_vm: Option<TlsVmInfo>,
path: String,
exe: DirEntryItem,
args: Option<ExeArgs>,
envs: Option<ExeArgs>,
) -> ! {
logln!("entry point: {}", entry);

let path = exe.full_path();

let args = args.map(|a| args::Args::new(a));
let envs = envs.map(|e| args::Args::new(e));

Expand Down Expand Up @@ -538,9 +541,12 @@ impl Task {
helper.write(argp.len()); // int argc
}

// !!! Prevent memory leaks as we are not running destructors here!
drop(envp);
drop(argp);
drop(tls_vm);
drop(path);
drop(exe);

logln!("exec user stack: {:#x}", helper.current());
assert_eq!(helper.current() % 16, 0);
Expand Down
5 changes: 4 additions & 1 deletion cykusz-rs/src/kernel/fs/dirent.rs
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,10 @@ impl Cacheable<CacheKey> for DirEntry {
data.parent = None;
}

fn notify_used(&self) {}
fn notify_used(&self) {
let data = self.write();
logln!("mark used: {}", data.name);
}

fn deallocate(&self, _me: &CacheItem<CacheKey, DirEntry>) {
logln!("deallocate {}", _me.data.lock().name);
Expand Down
15 changes: 10 additions & 5 deletions cykusz-rs/src/kernel/fs/ext2/inode.rs
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ use crate::kernel::fs::ext2::disk;
use crate::kernel::fs::ext2::idata::INodeData;
use crate::kernel::fs::ext2::Ext2Filesystem;
use crate::kernel::fs::filesystem::Filesystem;
use crate::kernel::fs::icache;
use crate::kernel::fs::icache::{INodeItem, INodeItemStruct};
use crate::kernel::fs::inode::INode;
use crate::kernel::fs::pcache::{
Expand Down Expand Up @@ -153,19 +154,19 @@ impl LockedExt2INode {
if hl_count > 0 {
let mut writer = self.d_inode_writer();

logln_disabled!("dec_hl_count {}", hl_count - 1);
logln!("dec_hl_count {}", hl_count - 1);
writer.dec_hl_count();

if hl_count == 1 {
writer.set_deletion_time(crate::kernel::time::unix_timestamp() as u32);
}

logln_disabled!("unref {} hl: {}", id, hl_count - 1);
logln!("unref {} hl: {}", id, hl_count - 1);
}

if hl_count == 1 {
//It's 0 after decrement
logln_disabled!("drop from cache {}", id);
logln!("drop from cache {}", id);
self.ext2_fs().drop_from_cache(id);
}
}
Expand Down Expand Up @@ -683,7 +684,7 @@ impl INode for LockedExt2INode {
}

fn unlink(&self, name: &str) -> Result<()> {
logln_disabled!("unlink started");
logln!("unlink started sc");

let fs = self.ext2_fs();
let _lock = fs.dir_lock();
Expand All @@ -708,6 +709,8 @@ impl INode for LockedExt2INode {
if inode.ftype()? == FileType::Dir {
return Err(FsError::NotFile);
}

logln!("inode sc: {}", inode.strong_count());
}

let mut iter = DirEntIter::new(self.self_ref());
Expand All @@ -726,6 +729,8 @@ impl INode for LockedExt2INode {
logln!("{:?}", e);
}

icache::cache().print_stats();

Ok(())
}

Expand Down Expand Up @@ -1087,6 +1092,6 @@ impl INode for LockedExt2INode {
}

fn debug(&self) {
logln_disabled!("INode debug: {:?}", self.read_debug(32).d_inode());
logln!("INode debug: {:?}", self.read().d_inode());
}
}
2 changes: 1 addition & 1 deletion cykusz-rs/src/kernel/fs/ext2/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -150,7 +150,7 @@ impl Ext2Filesystem {
}

pub fn free_inode(&self, inode: &LockedExt2INode) {
logln_disabled!("Free inode: {}", inode.read_debug(33).id());
logln!("Free inode: {}", inode.read_debug(33).id());

inode.write_debug(17).free_blocks(self);

Expand Down
2 changes: 1 addition & 1 deletion cykusz-rs/src/kernel/syscall/sys.rs
Original file line number Diff line number Diff line change
Expand Up @@ -548,7 +548,7 @@ pub fn sys_unlink(at: u64, path: u64, path_len: u64, flags: u64) -> SyscallResul

let path = Path::new(path_str);

logln4!("sys_unlink: {}, flags: {}", path.str(), flags);
logln!("sys_unlink: {}, flags: {}", path.str(), flags);

let file = get_dir_entry(at, Some(path), LookupMode::None, true)?;

Expand Down
7 changes: 5 additions & 2 deletions cykusz-rs/src/kernel/task/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -210,6 +210,9 @@ impl Task {
self.filetable().close_on_exec();
self.filetable().debug();

// !!! Prevent memory leak as we are not running destructors here!
drop(vm);

unsafe {
// EXEC!
self.arch_task_mut().exec(
Expand All @@ -218,7 +221,7 @@ impl Task {
&elf_hdr,
self.vm(),
tls_vm,
exe.full_path(),
exe,
Some(args),
envs,
)
Expand Down Expand Up @@ -781,6 +784,6 @@ impl Task {

impl Drop for Task {
fn drop(&mut self) {
logln4!("drop task {}", self.tid());
logln!("drop task {}", self.tid());
}
}
6 changes: 6 additions & 0 deletions cykusz-rs/src/kernel/task/vm.rs
Original file line number Diff line number Diff line change
Expand Up @@ -107,6 +107,7 @@ impl MMapedFile {

impl Drop for MMapedFile {
fn drop(&mut self) {
logln!("drop mapped file");
for (&addr, mapping) in self.active_mappings.iter() {
let uaddr: UserAddr = addr.into();

Expand Down Expand Up @@ -932,6 +933,11 @@ impl Default for VM {
}
}

impl Drop for VM {
fn drop(&mut self) {
}
}

bitflags! {
pub struct PageFaultReason: usize {
const PRESENT = 0b1;
Expand Down
6 changes: 3 additions & 3 deletions disk-scripts/fsck_disk.sh
Original file line number Diff line number Diff line change
Expand Up @@ -5,8 +5,8 @@ set -e
losetup -D
losetup -P /dev/loop0 disk.img

fsck.ext2 /dev/loop0p1 -f -v -n
fsck.ext2 /dev/loop0p2 -f -v -n
fsck.ext2 /dev/loop0p3 -f -v -n
fsck.ext2 /dev/loop0p1 -f -v
fsck.ext2 /dev/loop0p2 -f -v
fsck.ext2 /dev/loop0p3 -f -v

losetup -D

0 comments on commit 64ee581

Please sign in to comment.