On 10/23/19 1:01 PM, Matthew Malcomson wrote: > Hi Martin, Hello.
> > I'm getting close to putting up a patch series that I believe could go > in before stage1 close. > > I currently have to do testing on sanitizing the kernel, and track down > a bootstrap comparison diff in the code handling shadow-stack cleanup > during exception unwinding. > > I just thought I'd answer these questions below to see if there's > anything I extra could to do to make reviewing easier. I welcome that approach. > > On 23/09/19 09:02, Martin Liška wrote: >> Hi. >> >> As mentioned in the next email thread, there are main objectives >> that will help me to make a proper patch review: >> >> 1) Make first libsanitizer merge from trunk, it will remove the need >> of the backports that you made. Plus I will be able to apply the >> patchset on the current master. > Done >> 2) I would exclude the setjmp/longjmp - these should be upstreamed first >> in libsanitizer. > > Will exclude in the patch series, upstreaming under progress > (https://reviews.llvm.org/D69045) > >> 3) I would like to see a two HWASAN options that will clearly separate the >> 2 supported modes: TBI without MTE and MTE. Here I would appreciate to >> have >> a compiler farm machine with TBI which we can use for testing. > > I went back and looked at clang to see that it uses > `-fsanitize=hwaddress` and `-fsanitize=memtag`, which are completely > different options. > > I'm now doing the same, with the two sanitizers just using similar code > paths. > > In fact, I'm not going to have the MTE instrumentation ready by the end > of stage1, so my aim is to just put the `-fsanitize=hwaddress` sanitizer > in, but send some outline code to the mailing list to demonstrate how > `-fsanitize=memtag` would fit in. As well here. That will make it easier to merge -fsanitize=hwaddress first. > > > ## w.r.t. a compiler farm machine with TBI > > Any AArch64 machine has this feature. However in order to use the > sanitizer the kernel needs to allow "tagged pointers" in syscalls. If so, then it will be very easy to grab a machine and run 5.4 kernel in it. So I'll will be able to test the patches. > > The kernel has allowed these tagged pointers in syscalls (once it's been > turned on with a relevant prctl) in mainline since 5.4-rc1 (i.e. the > start of this month). > > My testing has been on a virtual machine with a mainline kernel built > from source. > > Given that I'm not sure how you want to proceed. > Could we set up a virtual machine on the compiler farm? > > >> 4) About the BUILTIN expansion: you provided a patch for couple of them. My >> question >> is whether the list is complete? > > The list of BUILTINs was nowhere near complete at the time I posted the > RFC patches. > > Since then I've added features and correspondingly added BUILTINs. > > Now I believe I've added all the BUILTIN's into sanitizer.def this > sanitizer will need. > >> 5) I would appreciate the patch set to be split into less logical parts, e.g. >> libsanitizer changes; option introduction; stack variable handling >> (colour/uncolour/alignment); >> hwasan pass and other GIMPLE-related changes; RTL hooks, new RTL >> instructions and expansion changes. >> > > Will do! Great. Thanks, Martin > >> Thank you, >> Martin >> >> >
