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

Reachable unwrap panic in sys_munmap() at legacy.rs #141

Open
Marsman1996 opened this issue Oct 8, 2024 · 5 comments
Open

Reachable unwrap panic in sys_munmap() at legacy.rs #141

Marsman1996 opened this issue Oct 8, 2024 · 5 comments
Assignees

Comments

@Marsman1996
Copy link

Describe the bug

There is a reachable unwrap panic in sys_munmap() caused by aligning large len (i.e., 0xffffffffffffffff) of a munmap syscall.

let layout = Layout::from_size_align(len, 8).unwrap();

To Reproduce

  1. Compile a program which calls munmap:
#include <errno.h>
#include <stdio.h>
#include <stdlib.h>
#include <sys/mman.h>
#include <unistd.h>

int main()
{
    void *addr = mmap(NULL, 4096, 0x3, MAP_PRIVATE | MAP_ANONYMOUS, -1, 0);
    munmap(addr, 0xffffffffffffffff);

    return 0;
}

With following features:

alloc 
irq
musl
multitask
fs
pipe
poll
rtc
signal
virtio-9p
  1. Run the compiled program in RuxOS.

Expected behavior

RuxOS reports panic and is terminated.

Environment

  • RuxOS version: main b1f880b
  • ubuntu:22.04 in Docker
  • 11th Gen Intel(R) Core(TM) i7-1165G7 @ 2.80GHz

Logs

[1728388549.205894 0:2 ruxos_posix_api::imp::mmap::legacy:67] sys_munmap <= start: 0xffffff80003fe000, len: 18446744073709551615
[1728388549.206739 0:2 ruxruntime::lang_items:14] panicked at api/ruxos_posix_api/src/imp/mmap/legacy.rs:72:54:
called `Result::unwrap()` on an `Err` value: LayoutError
[1728388549.207796 0:2 ruxhal::platform::x86_pc::misc:16] Shutting down...
@Marsman1996
Copy link
Author

Currently RuxOS will panic when page fault happening. For example, program access an invalid memory region.

I'm wondering is it a feature, or will RuxOS handle it in the future?

@coolyjg
Copy link
Contributor

coolyjg commented Oct 8, 2024

Thanks for your bug report for RuxOS!
It is not a feature or anything. We directly used unwrap() for Layout::from_size_align and did not handle Err value, and finally caused this issue. @ken4647
By the way, there are some suggestions for this issue:

  1. munmap should fail and return error number when some part of the region being unmapped is not part of the currently valid address space(see system call manual).
  2. sys_munmap panics due to large len, which should be checked.

@Marsman1996
Copy link
Author

Marsman1996 commented Oct 8, 2024

Thank you for your quick reply!

And my question is actually about this:

panic!(
"Kernel #PF @ {:#x}, fault_vaddr={:#x}, error_code={:#x}:\n{:#x?}",
tf.rip, vaddr, tf.error_code, tf,
);

When RuxOS accesses an invalid memory and causes a page fault, it will panic.

Will it be handled in the future?
Since it is an unikenel and run one program at once, it seems ok to panic directly?

@coolyjg
Copy link
Contributor

coolyjg commented Oct 8, 2024

When accessing invalid memory, program behavior is undefined, so panic is a safe implementation. (I'm not sure whether it will be handled in the future.)

@Marsman1996
Copy link
Author

And a similarly unwrap panic for sys_mmap() at legacy.rs.

let layout = Layout::from_size_align(len, 8).unwrap();

If we run following program which calls mmap with large size (i.e., 0x7ffffffffffffffe)

#include <errno.h>
#include <stdio.h>
#include <stdlib.h>
#include <sys/mman.h>
#include <unistd.h>

int main()
{
    mmap(NULL, 0x7ffffffffffffffe, 0x3, MAP_PRIVATE | MAP_ANONYMOUS, -1, 0);

    return 0;
}

With following features:

alloc 
irq
musl
multitask
pipe
poll
rtc
signal
virtio-9p

logs like

[1728436128.205115 0:2 ruxmusl::x86_64:10] x86 syscall <= syscall_name: MMAP
[1728436128.205642 0:2 ruxos_posix_api::imp::mmap::legacy:32] sys_mmap <= start: 0x0, len: 9223372036854775806, fd: -1
[1728436128.206396 0:2 ruxruntime::lang_items:14] panicked at api/ruxos_posix_api/src/imp/mmap/legacy.rs:44:54:
called `Result::unwrap()` on an `Err` value: LayoutError
[1728436128.207400 0:2 ruxhal::platform::x86_pc::misc:16] Shutting down...

@ken4647 ken4647 self-assigned this Dec 3, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants