sysprog21 / simplefs

A simple native file system for Linux kernel
Other
362 stars 91 forks source link

Fix segment fault exe symbolic link then run ls cmd #35

Closed RoyWFHuang closed 8 months ago

RoyWFHuang commented 8 months ago

Description

When create a symbolic link and then execute 'ls' command, this will make the segmentation fault in user space

Root cause

By checking kernel log, we can see that "usercopy: Kernel memory exposure attempt detected from SLUB object 'simplefs_cache' (offset 0, size 5)!", and by tracing kernel call, we also find call "readlink_copy" fail

In readlink_copy, this will copy data to user space, and the message shows kernel memoery exposure.

Fix solution

By kernel document https://docs.kernel.org/core-api/memory-allocation.html?highlight=kmem_cache_create

"kmem_cache_create() or kmem_cache_create_usercopy() before it can be used. The second function should be used if a part of the cache might be copied to the userspace"

and readlink will copy the target name from simplefs inode link(inode->i_link) to the user space, so we replace kmem_cache_create to kmem_cache_create_usercopy

How has this been tested:

run make check

Testing cmd: ln file hdlink...Success Testing cmd: mkdir dir/dir...Success Testing cmd: ln -s file symlink...Success Testing cmd: ls -lR...Success Testing cmd: mkdir len_of_name_of_this_dir_is_29...Success Testing cmd: touch len_of_name_of_the_file_is_29...Success Testing cmd: ln -s dir len_of_name_of_the_link_is_29...Success Testing cmd: echo abc > file...Success Testing cmd: dd if=/dev/zero of=file bs=1M count=12 status=none...dd: error writing 'file': File too large Check if exist: drwxr-xr-x 3 dir...Success Check if exist: -rw-r--r-- 2 file...Success Check if exist: -rw-r--r-- 2 hdlink...Success Check if exist: drwxr-xr-x 2 dir...Success Check if exist: lrwxrwxrwx 1 symlink...Success

Close #30

jserv commented 8 months ago

Can you provide the relevant test case?

RoyWFHuang commented 8 months ago

I run the "make check" and also use

#!/bin/bash
touch test/file1
cd test
ln -s file1 symlink
ls -lR

this bash script for test

jserv commented 8 months ago

I run the "make check" and also use [...]

Extend script/test.sh for automated tests.

jserv commented 8 months ago

By the way, after resolving this issue, it is time to activate GitHub Actions for automated tests. See rv32emu workflows for example.

RoyWFHuang commented 8 months ago

I think it no needed to add the script into the scritp/test.sh file, beacuse this script is extracted from it

# create file
test_op 'touch file'
.....(skip)

# symbolic link
test_op 'ln -s file symlink'

# list directory contents
test_op 'ls -lR'

it's very similar with script/test.sh, I just skip the "touch a lot of file" testing

jserv commented 8 months ago

I think it no needed to add the script into the scritp/test.sh file, beacuse this script is extracted from it

Then, we can move forward to GitHub Actions backed automations.