On Mon, Oct 11, 2021 at 04:59:28PM -0700, Linus Torvalds wrote: > On Mon, Oct 11, 2021 at 2:08 PM Catalin Marinas <catalin.marinas@xxxxxxx> wrote: > > +#ifdef CONFIG_ARM64_MTE > > +#define FAULT_GRANULE_SIZE (16) > > +#define FAULT_GRANULE_MASK (~(FAULT_GRANULE_SIZE-1)) > > [...] > > > If this looks in the right direction, I'll do some proper patches > > tomorrow. > > Looks fine to me. It's going to be quite expensive and bad for caches, > though. > > That said, fault_in_writable() is _supposed_ to all be for the slow > path when things go south and the normal path didn't work out, so I > think it's fine. > > I do wonder how the sub-page granularity works. Is it sufficient to > just read from it? For arm64 MTE and I think SPARC ADI, just reading should be sufficient. There is CHERI in the long run, if it takes off, where the user can set independent read/write permissions and uaccess would use the capability rather than a match-all pointer (hence checked). > Because then a _slightly_ better option might be to > do one write per page (to catch page table writability) and then one > read per "granule" (to catch pointer coloring or cache poisoning > issues)? > > That said, since this is all preparatory to us wanting to write to it > eventually anyway, maybe marking it all dirty in the caches is only > good. It depends on how much would be written in the actual copy. For significant memcpy on arm CPUs, write streaming usually kicks in and the cache dirtying is skipped. This probably matters more for copy_page_to_iter_iovec() than the btrfs search ioctl. Apart from fault_in_pages_*(), there's also fault_in_user_writeable() called from the futex code which uses the GUP mechanism as the write would be destructive. It looks like it could potentially trigger the same infinite loop on -EFAULT. For arm64 MTE, we get away with this by disabling the tag checking around the arch futex code (we did it for an unrelated issue - we don't have LDXR/STXR that would run with user permissions in kernel mode like we do with LDTR/STTR). I wonder whether we should actually just disable tag checking around the problematic accesses. What these callers seem to have in common is using pagefault_disable/enable(). We could abuse this to disable tag checking or maybe in_atomic() when handling the exception to lazily disable such faults temporarily. A more invasive change would be to return a different error for such faults like -EACCESS and treat them differently in the caller. -- Catalin
