<p style="white-space: pre-wrap; word-wrap: break-word;">I think the commit in its current form creates a (security) problem</p><p>Patch set 1:<span style="border-radius: 3px; display: inline-block; margin: 0 2px; padding: 4px;background-color: #ffd4d4;">Code-Review -1</span></p><p><a href="https://review.coreboot.org/28394">View Change</a></p><p>2 comments:</p><ul style="list-style: none; padding: 0;"><li style="margin: 0; padding: 0;"><p><a href="https://review.coreboot.org/#/c/28394/1//COMMIT_MSG">Commit Message:</a></p><ul style="list-style: none; padding: 0;"><li style="margin: 0; padding: 0 0 0 16px;"><p style="margin-bottom: 4px;"><a href="https://review.coreboot.org/#/c/28394/1//COMMIT_MSG@10">Patch Set #1, Line 10:</a> <code style="font-family:monospace,monospace">So make this change.</code></p><p style="white-space: pre-wrap; word-wrap: break-word;">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.</p></li></ul></li><li style="margin: 0; padding: 0;"><p><a href="https://review.coreboot.org/#/c/28394/1/src/arch/riscv/include/vm.h">File src/arch/riscv/include/vm.h:</a></p><ul style="list-style: none; padding: 0;"><li style="margin: 0; padding: 0 0 0 16px;"><p style="margin-bottom: 4px;"><a href="https://review.coreboot.org/#/c/28394/1/src/arch/riscv/include/vm.h@41">Patch Set #1, Line 41:</a> <code style="font-family:monospace,monospace">#define DEFINE_MPRV_READ(name, type, insn)                              \</code></p><p style="white-space: pre-wrap; word-wrap: break-word;">I suggest the following:</p><ul><li>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.</li><li>#define DEFINE_MPRV_READ(name, type, insn) DEFINE_MPRV_READ_FLAGS(name, type, insn, 0)</li><li>#define DEFINE_MPRV_READ_MXR(name, type, insn) DEFINE_MPRV_READ_FLAGS(name, type, insn, MSTATUS_MXR)</li><li>Add a separate MXR-enabled read function for bytes.</li></ul></li></ul></li></ul><p>To view, visit <a href="https://review.coreboot.org/28394">change 28394</a>. To unsubscribe, or for help writing mail filters, visit <a href="https://review.coreboot.org/settings">settings</a>.</p><div itemscope itemtype="http://schema.org/EmailMessage"><div itemscope itemprop="action" itemtype="http://schema.org/ViewAction"><link itemprop="url" href="https://review.coreboot.org/28394"/><meta itemprop="name" content="View Change"/></div></div>

<div style="display:none"> Gerrit-Project: coreboot </div>
<div style="display:none"> Gerrit-Branch: master </div>
<div style="display:none"> Gerrit-MessageType: comment </div>
<div style="display:none"> Gerrit-Change-Id: I19519782fe791982a8fbd48ef33b5a92a3c48bfc </div>
<div style="display:none"> Gerrit-Change-Number: 28394 </div>
<div style="display:none"> Gerrit-PatchSet: 1 </div>
<div style="display:none"> Gerrit-Owner: Xiang Wang <wxjstz@126.com> </div>
<div style="display:none"> Gerrit-Reviewer: Jonathan Neuschäfer <j.neuschaefer@gmx.net> </div>
<div style="display:none"> Gerrit-Reviewer: Philipp Hug <philipp@hug.cx> </div>
<div style="display:none"> Gerrit-Reviewer: Shawn Chang <citypw@gmail.com> </div>
<div style="display:none"> Gerrit-Reviewer: build bot (Jenkins) <no-reply@coreboot.org> </div>
<div style="display:none"> Gerrit-Comment-Date: Thu, 30 Aug 2018 12:07:45 +0000 </div>
<div style="display:none"> Gerrit-HasComments: Yes </div>
<div style="display:none"> Gerrit-HasLabels: Yes </div>