Bug: 在特定的内存布局下，page_tables::translated_refmut 函数会导致 其返回的引用在内存中被截断的情况

[Amaranterre]

Bug: 在特定的内存布局下，page_tables::translated_refmut 函数会导致 其返回的引用在内存中被截断的情况

/// Translate a ptr[u8] array through page table and return a mutable reference of T
pub fn translated_refmut<T>(token: usize, ptr: *mut T) -> &'static mut T {
    //trace!("into translated_refmut!");
    let page_table = PageTable::from_token(token);
    let va = ptr as usize;
    //trace!("translated_refmut: before translate_va");
    page_table
        .translate_va(VirtAddr::from(va))
        .unwrap()
        .get_mut()
}

对于page_tables.rs中的translated_refmut函数，其直接从va中获得相应的pa并以这个pa作为实例T的起始地址。实现了直接从物理地址空间修改实例T的内容。

这种写法的问题在于，可能存在这种情况，即尽管实例T的对应的数据内存在虚拟地址空间上是连续的，但这段内存在物理地址空间上却不能保证是连续的，因为这里直接在内核态修改物理地址空间，因此可能出现实例T被截断的情况。比如一个4字节的整型变量的起始地址是在虚拟页v(n)的最后一个字节处，当虚拟页v(n)和v(n+1)对应的物理页不连续时，这时我们修改这个变量，那么这个修改就很有可能不会反映在v(n+1)上。

以下我对这个问题的复现。在CH5的系统调用中，sys_waitpid使用了translated_refmut函数来达到修改用户态中的 exit_code的目的。在我的复现中，我将sys_waitpid的返回值设定为-100，且将存储这个返回值的变量的起始地址设置在了当前虚拟页的最后一个Byte位置；当我们在用户态中打印这个变量时，得到的却是156。很明显，这是因为-100 对应的补码*11111111 11111111 11111111 10011100*被截断成了*00000000 00000000 00000000 10011100*,即156。

// ch5_trans_test.rs
#![no_std]
#![no_main]

#[macro_use]
extern crate user_lib;

// use core::slice::range;

use user_lib::{exit, fork, sbrk, wait, yield_};

#[no_mangle]
pub fn main() -> i32 {
    let pid = fork();
    if pid == 0 {
        // attention: the exit_code returned by this subprocess will cause bug
        println!("I am child!");
        let mut time = 0;
        while time < 1000000000 {
            time += 1;
        }
        exit(-100);
    } else {
        println!("forked child pid = {}", pid);
    }

    unsafe {
        // get the virtual address of the last byte
        // in current virtual page, stored it into `exit_code_ptr`
        let mut exit_code: i32 = 0;
        let mut exit_code_ptr = &mut exit_code as *mut i32 as usize;
        exit_code_ptr = (exit_code_ptr / 4096 + 1) * 4096 - 1;

        let pid = fork();
        if pid == 0 {
            // the porpose of this process is creating more physical pages,
            // so the physical pages in parent process will not be continuous
            // we also may keep it alive so its pa won't be recycled
            sbrk(4096 * 5);
            let mut time = 0;
            while time < 1000000000 {
                time += 1;
                yield_();
            }
            exit(-100);
        } else {
            yield_();
        }
        
        // create a new virtual pages, otherwise will case page fault
        // attention: some part of `exit code` will be in this new vp and 
        //            some will be in the original vp
        sbrk(4096);
        
        // the variable which its bytes was continuous in vp bu not continuous in pp
        // which if we modify it will cause the bug I mentioned in this issue
        let exit_code1 : &mut i32 = &mut *(exit_code_ptr as *mut i32);

        wait(exit_code1);
        // `wait` should return -100 but what we got is 156, and
        // 156 = 00000000 00000000 00000000 10011100
        //-100 = 11111111 11111111 11111111 10011100
        // since we use little endian and  two's complement, obviously first three bytes of
        // "-100" was truncated in this case because of physical page incontinuity 
        // which we don't expect !
        println!("exit code: {}", exit_code1);
    }
    println!("forktest pass.");
    0
}

目前仅在CH5分支上进行了测试。

程序为了构造复现条件使用了unsafe模式，实际上不使用unsafe也可以复现，关键是要被修改的变量出现在页尾，且这相邻的两个虚拟页对应的物理页不连续。

[hky1999]

Yep, you are absolutely right.

Indeed, translated_str shares the same bug with translated_refmut.

In fact, under current code implementation, translated_byte_buffer is the only available method to splice discontinuous physical memory.

we can use functions copy_from_user and copy_to_user to decorate these operations, like Linux do in uaccess.h

[Amaranterre]

@hky1999 thanks for replying! may be copy_from_user and copy_from_user can be implemented in our code in future?