Hi, [...] > If that's the case, then the patch is alright. I was mostly worried > about messing with userspace memory of a random process. > > > If we put on our paranoia hats, the biggest problem with borrowing > > userspace's mm is that it gives them temporary insight into whatever > > we place into that mm. We don't expose any data, unless by error... > > Not sure how much effort we want to put on making the selftests paranoia > > proof, but that (and the surety of cleaning up afterwards) would be a > > good argument for creating a temporary mm for our use. > > I don't think it's really a secret what the selftest puts in that memory > anyway (assuming normal test operation). > > The only problem I can see with using userspace's mm at the moment > (paranoia-wise) is that we only lock the mm for the vma_lookup() check > [1], meaning there's a time-of-check-time-of-use situation. IFF > userspace can somehow unmap the obj from its side after that check is > done, this can potentially mess with kernel memory. I have no earthly > idea if this can be really abused though, so it might not even be a real > issue. > > Besides, to achieve that, a malicious process would have to win the > kthread active_mm lottery so its mm is used for the selftest, then guess > the address of the mmapped object (it's technically logged as debug, but > the loglevel might not be set as such), and then race with the kthread > so the object is unmapped before use. So a lot of stars have to align. [...] > [1]: gem/selftests/i915_gem_mman.c:923 Could a malicious process get all necessary information from logs (if available)? Successful guesswork and winning races seem unlikely to happen at the same time, but still possible. That being said, intentionally brute forcing search of these values seems even less likely to complete in the small window when this test runs, so: Reviewed-by: Krzysztof Karas <krzysztof.karas@xxxxxxxxx> Best Regards, Krzysztof
