On 2025-03-07 at 12:31:25 GMT, Chris Wilson wrote: > Quoting Mikolaj Wasiak (2025-03-07 08:44:29) > > Hi Krzysztof, > > > > On 2025-03-05 at 17:31:49 +0100, Krzysztof Niemiec wrote: > > > Don't we run into the same issue as in V1, meaning we use an unknown > > > current->active_mm (since we run in a kthread, and cannot control it) to > > > use as the current->mm? Maybe a better approach would be to create a new > > > mm for the duration of the test, similarly to how the patch Janusz > > > mentioned does it? (51104c19d857) > > > > As per discussion with Chris, using active_mm is the correct way of > > enabling current->mm in kthread. On the other hand it may also expose > > issues with underlying tests because they didn't previously run on such > > hardware. I think potential fixes to those tests should be addressed in > > separate patch. > > We've looked at the tests, and they should all be finding unused space > in the mm and cleaning up after themselves... > 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. If that is not something we consider a problem, then: Reviewed-by: Krzysztof Niemiec <krzysztof.niemiec@xxxxxxxxx> [1]: gem/selftests/i915_gem_mman.c:923 > -Chris Thanks Krzysztof
