On Fri, Nov 20, 2020 at 11:24 AM Eric W. Biederman <ebiederm@xxxxxxxxxxxx> wrote: > > Peter Collingbourne <pcc@xxxxxxxxxx> writes: > > > On Fri, Nov 20, 2020 at 9:44 AM Eric W. Biederman <ebiederm@xxxxxxxxxxxx> wrote: > >> > >> Peter Collingbourne <pcc@xxxxxxxxxx> writes: > >> > >> > The kernel currently clears the tag bits (i.e. bits 56-63) in the fault > >> > address exposed via siginfo.si_addr and sigcontext.fault_address. However, > >> > the tag bits may be needed by tools in order to accurately diagnose > >> > memory errors, such as HWASan [1] or future tools based on the Memory > >> > Tagging Extension (MTE). > >> > > >> > We should not stop clearing these bits in the existing fault address > >> > fields, because there may be existing userspace applications that are > >> > expecting the tag bits to be cleared. Instead, introduce a flag in > >> > sigaction.sa_flags, SA_EXPOSE_TAGBITS, and only expose the tag bits > >> > there if the signal handler has this flag set. > >> > > >> > [1] http://clang.llvm.org/docs/HardwareAssistedAddressSanitizerDesign.html > >> > >> For the generic bits: > >> Acked-by: "Eric W. Biederman" <ebiederm@xxxxxxxxxxxx> > > > > Thanks for the review. > > > >> Some of the arm bits look wrong. There are a couple of cases where > >> it looks like you are deliberately passing an untagged address into > >> functions that normally take tagged addresses. > >> > >> It might be a good idea to have a type distinction between the two. > >> Perhaps "(void __user *)" vs "(unsigned long)" so that accidentally > >> using the wrong one generates a type error. > >> > >> I don't think I am really qualified to review all of the arm details, > >> and I certainly don't want to be in the middle of any arm bugs this > >> code might introduce. If you will split out the generic bits of this > >> patch I will take it. The this whole thing can be merged into the arm > > > > Okay, I'll do that. > > Thank you. Sent out v21 with the generic parts split out. > >> tree and you can ensure the arm bits are correct. > > > > So you want Catalin to merge your signal-for-v5.11 branch with the > > generic patches into the arm64 tree and then add the arm64-specific > > patch on top? Works for me. > > I think that is what we discussed. Unless he has objections I would > prefer that. I based the branch on -rc3 in hopes that I would > not cause problems for his process. > > In the cases where I was confused you probably want comments describing > why the tag bits are being cleared. It may be obvious when you know the > architecture intimately but it certainly was not for me. I certainly > don't know the details of arm64 well enough to understand the > architecture specific nuances. I think that makes sense. I've added comments to those two cases. Peter
