[coreboot-gerrit] Change in coreboot[master]: riscv: Fix the definition of DEFINE_MPRV_READ

[Jonathan Neuschäfer (Code Review)]

Jonathan Neuschäfer has posted comments on this change. ( https://review.coreboot.org/28394 )

Change subject: riscv: Fix the definition of DEFINE_MPRV_READ
......................................................................


Patch Set 1: Code-Review-1

(2 comments)

I think the commit in its current form creates a (security) problem

https://review.coreboot.org/#/c/28394/1//COMMIT_MSG
Commit Message:

https://review.coreboot.org/#/c/28394/1//COMMIT_MSG@10
PS1, Line 10: So make this change.
As far as I understand it, unprivileged code could use this to read from execute-only pages by reading at misaligned addresses and triggering the misaligned memory access handler. To avoid this problem, MXR should probably only be used to read instructions, but not to read data.


https://review.coreboot.org/#/c/28394/1/src/arch/riscv/include/vm.h
File src/arch/riscv/include/vm.h:

https://review.coreboot.org/#/c/28394/1/src/arch/riscv/include/vm.h@41
PS1, Line 41: #define DEFINE_MPRV_READ(name, type, insn)				\
I suggest the following:

- Rename DEFINE_MPRV_READ to DEFINE_MPRV_READ_FLAGS and add an extra parameter called "flags" which is OR'd to the mprv variable in the definition.
- #define DEFINE_MPRV_READ(name, type, insn) DEFINE_MPRV_READ_FLAGS(name, type, insn, 0)
- #define DEFINE_MPRV_READ_MXR(name, type, insn) DEFINE_MPRV_READ_FLAGS(name, type, insn, MSTATUS_MXR)
- Add a separate MXR-enabled read function for bytes.



-- 
To view, visit https://review.coreboot.org/28394
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings

Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I19519782fe791982a8fbd48ef33b5a92a3c48bfc
Gerrit-Change-Number: 28394
Gerrit-PatchSet: 1
Gerrit-Owner: Xiang Wang <wxjstz at 126.com>
Gerrit-Reviewer: Jonathan Neuschäfer <j.neuschaefer at gmx.net>
Gerrit-Reviewer: Philipp Hug <philipp at hug.cx>
Gerrit-Reviewer: Shawn Chang <citypw at gmail.com>
Gerrit-Reviewer: build bot (Jenkins) <no-reply at coreboot.org>
Gerrit-Comment-Date: Thu, 30 Aug 2018 12:07:45 +0000
Gerrit-HasComments: Yes
Gerrit-HasLabels: Yes
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.coreboot.org/pipermail/coreboot-gerrit/attachments/20180830/1bccaacc/attachment.html>