Quoting Tvrtko Ursulin (2018-09-06 10:31:21) > > On 06/09/2018 08:00, Chris Wilson wrote: > > Quoting Tvrtko Ursulin (2018-09-05 15:25:44) > >> From: Lionel Landwerlin <lionel.g.landwerlin@xxxxxxxxx> > >> > >> Verifies that the kernel programs slices correctly based by reading > >> the value of PWR_CLK_STATE register or MI_SET_PREDICATE on platforms > >> before Cannonlake. > >> > >> v2: Add subslice tests (Lionel) > >> Use MI_SET_PREDICATE for further verification when available (Lionel) > >> > >> v3: Rename to gem_ctx_rpcs (Lionel) > >> > >> v4: Update kernel API (Lionel) > >> Add 0 value test (Lionel) > >> Exercise invalid values (Lionel) > >> > >> v5: Add perf tests (Lionel) > >> > >> v6: Add new sysfs entry tests (Lionel) > >> > >> v7: Test rsvd fields > >> Update for kernel series changes > >> > >> v8: Drop test_no_sseu_support() test (Kelvin) > >> Drop drm_intel_*() apis (Chris) > >> > >> v9: by Chris: > >> Drop all do_ioctl/do_ioctl_err() > >> Use gem_context_[gs]et_param() > >> Use gem_read() instead of mapping memory > >> by Lionel: > >> Test dynamic sseu on/off more > >> > >> Tvrtko Ursulin: > >> > >> v10: > >> * Various style tweaks and refactorings. > >> * New test coverage. > > > > I didn't notice any testing of: > > - param->size > > It exists in test_invalid_args. > > > - feeding garbage into param->value user pointer (always cleared before > > use, perhaps just poison instead), along with abusive pointers. > > Also in test_invalid_args - but only the null pointer. I can add an > unmapped or read-only one. I think passing a pointer that starts valid and crosses into an unmapped page is essential. That makes sure we are using copy_from_user correctly. Another trivial one is param->value = -param->size + 1. As for the garbage, I wasn't sure (because I'm fully cogniscent of the sseu responses) if we would not interpret 0 as a valid response. If you are happy that we can differentiate an output 0 after passing in 0, that's all fine (as opposed to us forgetting to fill a value along some path). > > E.g., how does the code fare if we pass in an unfaulted GGTT mmap? > > Would not fare well. :I It would be best to be able to reject them but > how? We'll hit the same problem in future other patches so to support > this, I think we need to refactor Do you want my patch to drop struct_mutex entirely here :) So then you have to add it where you need it, which is after the copy_from_user() so recursive faults are not a problem. -Chris _______________________________________________ Intel-gfx mailing list Intel-gfx@xxxxxxxxxxxxxxxxxxxxx https://lists.freedesktop.org/mailman/listinfo/intel-gfx
