Patch 5

[XiaojingZhu]

Add config RV_NO_EXEC_BIT. This config will clear the X bit in the page table entry for data and stack memory, while the kernel loading the application to the memory . So the corresponding process can be protected from code injection attacks.

[aswaterman]

Should this be a kernel config flag? Do other architecture ports handle this sort of thing the same way?

[XiaojingZhu]

Arch x86 support NX by default. Function setup_arch calls function x86_configure_nx, in x86_configure_nx will check if the cpu support NX(Execute Disable, which is corresponding to hardware DEP, Data Execution Protection), if yes, it will clear the corresponding bit of the page table entry. There are more configurations about NX in the x86 arch, more code about it in the directiory arch/x86. In the riscv, I did it more simply, only add a configuration RV_NO_EXEC_BIT.

[XiaojingZhu]

the rocket chip supports use of execute bit, so we can clear this bit in the page table entry.

[sorear]

I think the way it's supposed to work is that the stack is mapped executable iff the process image requires it, which is a bit in the ELF header(?) that ultimately gets set by the linker if any object file takes the address of a nested function (which requires generating an on-stack trampoline). Details are probably in the functions just mentioned.

BTW we're also missing ASLR support right now.

[aswaterman]

@XiaojingZhu right. My question is, why is it an option, rather than making it mandatory. The X bit in the RISC-V PTE is required for all RISC-V systems.

My understanding is in line with @sorear's.

[XiaojingZhu]

Yes, you are right, @aswaterman @sorear, I think we should remove the execute bit from micro VM_DATA_DEFAULT_FLAGS mandatory, for security.

While loading elf file, the kernel will add the execute bit for the code segment, according the elf file. in the mm/mmap.c, a piece of code like: flags = VM_DATA_DEFAULT_FLAGS | VM_ACCOUNT | mm->def_flags;

The VM_DATA_DEFAULT_FLAGS should not has execute bit by default. I had made a test, if the VM_DATA_DEFAULT_FLAGS has execute bit, a code injection attack will success. While I set the VM_DATA_DEFAULT_FLAGS without execute bit, the code injection attack failed.

[aswaterman]

OK, so I think you should revise this PR to remove the config option, and unconditionally remove VM_EXEC from VM_DATA_DEFAULT_FLAGS. Sound right to everyone?

@palmer-dabbelt should PRs be issued against riscv-next, or do you plan to back-port them?

[XiaojingZhu]

@aswaterman yes, I will submit right way.