fortanix / rust-sgx

The Fortanix Rust Enclave Development Platform
https://edp.fortanix.com
Mozilla Public License 2.0
433 stars 98 forks source link

sgxs-info: Fix bug with incomplete data writes in dump-mem #529

Closed jovanbulck closed 11 months ago

jovanbulck commented 1 year ago

Replace the use of stdout().write(&data) with stdout().write_all(&data) to always ensure complete data writes.

The original code would silently continue even after the data buffer was only partially written, producing incomplete and buggy SGXS memory dumps in practice.

See also:

Example of buggy behavior

This can be easily observed, e.g., as follows already in the 2nd page of an EDP-generated SGXS:

$ sgxs-info summary hello-world.sgxs 
      0-   efff Reg  r--  (data) meas=all
   f000-  34fff Reg  r-x  (data) meas=all
   ...

Before the fix in this PR the executable page is not entirely copied into the memory dump (the last 1446 bytes are silently skipped and are filled in by the next call to stdout().write(&data) from the next page):

$ sgxs-info dump-mem hello-world.sgxs > hello.dump
$ diff -y --suppress-common-lines <(xxd hello-world) <(xxd hello.dump) | head
00001a50: f862 0300 0000 0000 0800 0000 0000 0000  .b........ | 00001a50: f862 0300 0000 0000 0800 0c4c 0000 0000  .b........
00001a60: 8d47 0000 0000 0000 1063 0300 0000 0000  .G.......c | 00001a60: 0000 a867 0300 0000 0000 0800 0000 0000  ...g......
00001a70: 0800 0000 0000 0000 ae47 0000 0000 0000  .........G | 00001a70: 0000 0c4c 0000 0000 0000 c067 0300 0000  ...L......
00001a80: 2063 0300 0000 0000 0800 0000 0000 0000   c........ | 00001a80: 0000 0800 0000 0000 0000 0c4c 0000 0000  ..........
00001a90: d347 0000 0000 0000 3863 0300 0000 0000  .G......8c | 00001a90: 0000 d867 0300 0000 0000 0800 0000 0000  ...g......
00001aa0: 0800 0000 0000 0000 f447 0000 0000 0000  .........G | 00001aa0: 0000 0c4c 0000 0000 0000 f067 0300 0000  ...L......
00001ab0: 4863 0300 0000 0000 0800 0000 0000 0000  Hc........ | 00001ab0: 0000 0800 0000 0000 0000 0c4c 0000 0000  ..........
00001ac0: d347 0000 0000 0000 6063 0300 0000 0000  .G......`c | 00001ac0: 0000 0868 0300 0000 0000 0800 0000 0000  ...h......
00001ad0: 0800 0000 0000 0000 1a48 0000 0000 0000  .........H | 00001ad0: 0000 0c4c 0000 0000 0000 2068 0300 0000  ...L......
00001ae0: 7863 0300 0000 0000 0800 0000 0000 0000  xc........ | 00001ae0: 0000 0800 0000 0000 0000 cc4c 0000 0000  ..........

After the fix in this PR the page is indeed properly copied into the memory dump:

$ diff -y <(xxd hello-world) <(xxd hello.dump.fixed) | grep 00001a50 -A 9
00001a50: f862 0300 0000 0000 0800 0000 0000 0000  .b........   00001a50: f862 0300 0000 0000 0800 0000 0000 0000  .b........
00001a60: 8d47 0000 0000 0000 1063 0300 0000 0000  .G.......c   00001a60: 8d47 0000 0000 0000 1063 0300 0000 0000  .G.......c
00001a70: 0800 0000 0000 0000 ae47 0000 0000 0000  .........G   00001a70: 0800 0000 0000 0000 ae47 0000 0000 0000  .........G
00001a80: 2063 0300 0000 0000 0800 0000 0000 0000   c........   00001a80: 2063 0300 0000 0000 0800 0000 0000 0000   c........
00001a90: d347 0000 0000 0000 3863 0300 0000 0000  .G......8c   00001a90: d347 0000 0000 0000 3863 0300 0000 0000  .G......8c
00001aa0: 0800 0000 0000 0000 f447 0000 0000 0000  .........G   00001aa0: 0800 0000 0000 0000 f447 0000 0000 0000  .........G
00001ab0: 4863 0300 0000 0000 0800 0000 0000 0000  Hc........   00001ab0: 4863 0300 0000 0000 0800 0000 0000 0000  Hc........
00001ac0: d347 0000 0000 0000 6063 0300 0000 0000  .G......`c   00001ac0: d347 0000 0000 0000 6063 0300 0000 0000  .G......`c
00001ad0: 0800 0000 0000 0000 1a48 0000 0000 0000  .........H   00001ad0: 0800 0000 0000 0000 1a48 0000 0000 0000  .........H
00001ae0: 7863 0300 0000 0000 0800 0000 0000 0000  xc........   00001ae0: 7863 0300 0000 0000 0800 0000 0000 0000  xc........
jovanbulck commented 11 months ago

@jethrogb friendly ping if this small fix can be reviewed and merged?

raoulstrackx commented 11 months ago

The sgxs: Update link to format specification commit is unrelated. Can you create a separate PR for that?

jovanbulck commented 11 months ago

rebased and created a separate PR for the documentation link

raoulstrackx commented 11 months ago

bors r+

bors[bot] commented 11 months ago

Build succeeded!

The publicly hosted instance of bors-ng is deprecated and will go away soon.

If you want to self-host your own instance, instructions are here. For more help, visit the forum.

If you want to switch to GitHub's built-in merge queue, visit their help page.