On Wed, Oct 7, 2020 at 8:17 AM Christoph Hellwig <hch@xxxxxxxxxxxxx> wrote: > On Wed, Oct 07, 2020 at 02:45:39AM +0200, Jann Horn wrote: > > > I think arch_validate_prot() is still the right hook to validate the > > > protection bits. sparc_validate_prot() can iterate over VMAs with read > > > lock. This will, of course, require range as well to be passed to > > > arch_validate_prot(). > > > > In that case, do you want to implement this? > > Any reason to not just call arch_validate_prot after taking the mmap > lock? Then the next easy steps are: - change the signature of arch_validate_prot() to also accept a length or end parameter (because a start address without an end address is completely useless) - add a loop over the VMAs in that range in SPARC's arch_validate_prot() And then comes the annoying part: figure out what to do in that loop if the range is not fully covered by VMAs. To fully avoid changing the normal mprotect() ABI, you'd have to accept cases where parts of the range are unmapped - but hopefully nobody relies on that particular weirdness, so maybe you can just throw an error in that case? Even so, the normal error code for that is -ENOMEM, but arch_validate_prot() has a boolean return, so for that, you'd have to change the return type, too. I guess the cleanest approach might be to just validate up to the first gap and then return "everything's good" and rely on mprotect() bailing out on its own? Ah - at first I thought that you'd also have to deal with concurrent stack VMA expansion from find_expand_vma() (which we really should get rid of somehow sometime), but luckily that still at least holds the mmap lock in read mode, and here we hold it in write mode, so we don't have to worry about that. So I guess that'd be the way to go for this approach? Alright, I guess I'll send patches after all, hopefully after at least compile-testing them...
