Re: [PATCH v4 1/6] io_uring/mock: add basic infra for test mock files

[Pavel Begunkov <asml.silence@gmail.com>]

On 5/30/25 16:30, Jens Axboe wrote:
> On 5/30/25 9:11 AM, Pavel Begunkov wrote:
>> On 5/30/25 15:41, Jens Axboe wrote:
>>> On 5/30/25 8:26 AM, Pavel Begunkov wrote:
>>>> On 5/30/25 15:12, Pavel Begunkov wrote:
>>>>> On 5/30/25 15:09, Pavel Begunkov wrote:
>>>>>> On 5/30/25 14:28, Jens Axboe wrote:
>>>>>>> On 5/30/25 6:51 AM, Pavel Begunkov wrote:
>>>>>>>> diff --git a/init/Kconfig b/init/Kconfig
>>>>>>>> index 63f5974b9fa6..9e8a5b810804 100644
>>>>>>>> --- a/init/Kconfig
>>>>>>>> +++ b/init/Kconfig
>>>>>>>> @@ -1774,6 +1774,17 @@ config GCOV_PROFILE_URING
>>>>>>>>           the io_uring subsystem, hence this should only be enabled for
>>>>>>>>           specific test purposes.
>>>>>>>> +config IO_URING_MOCK_FILE
>>>>>>>> +    tristate "Enable io_uring mock files (Experimental)" if EXPERT
>>>>>>>> +    default n
>>>>>>>> +    depends on IO_URING && KASAN
>>>>>>>> +    help
>>>>>>>> +      Enable mock files for io_uring subststem testing. The ABI might
>>>>>>>> +      still change, so it's still experimental and should only be enabled
>>>>>>>> +      for specific test purposes.
>>>>>>>> +
>>>>>>>> +      If unsure, say N.
>>>>>>>
>>>>>>> As mentioned in the other email, I don't think we should include KASAN
>>>>>>> here.
>>>>>>
>>>>>> I disagree. It's supposed to give a superset of coverage, if not,
>>>>>> mocking should be improved. It might be seen as a nuisance that you
>>>>>> can't run it with a stock kernel, but that desire is already half
>>>>>> step from "let's enable it for prod kernels for testing", and then
>>>>>> distributions will start forcing it on, because as you said "People
>>>>>> do all sorts of weird stuff".
>>>>>
>>>>> The purpose is to get the project even more hardened / secure through
>>>>> elaborate testing, that would defeat the purpose if non test systems
>>>>> will start getting errors because of some mess up, let's say in the
>>>>> driver.
>>>>
>>>> Alternatively, it doesn't help with bloating, but tainting the kernel
>>>> might be enough to serve the purpose.
>>>
>>> I think taint or KASAN dependencies is over-reaching. It has nothing to
>>> do with KASAN, and there's absolutely zero reason for it to be gated on
>>> KASAN (or lockdep, or whatever). You're never going to prevent people
>>> from running this in odd cases, and I think it's a mistake to try and do
>>> that. If the thing is gated on CAP_SYS_ADMIN, then that's Good Enough
>>> imho.
>>>
>>> It'll make my life harder for coverage testing, which I think is reason
>>> enough alone to not have a KASAN dependency. No other test code in the
>>> kernel has unrelated dependencies like KASAN, unless they are related to
>>> KASAN. We should not add one here for some notion of preventing people
>>> from running it on prod stuff, in fact it should be totally fine to run
>>> on a prod kernel. Might actually be useful in some cases, to verify or
>>> test some behavior on that specific kernel, without needing to build a
>>> new kernel for it.
>>
>> commit 2852ca7fba9f77b204f0fe953b31fadd0057c936
>> Author: David Gow <davidgow@google.com>
>> Date:   Fri Jul 1 16:47:41 2022 +0800
>>
>>      panic: Taint kernel if tests are run
>>          Most in-kernel tests (such as KUnit tests) are not supposed to run on
>>      production systems: they may do deliberately illegal things to trigger
>>      errors, and have security implications (for example, KUnit assertions
>>      will often deliberately leak kernel addresses).
>>          Add a new taint type, TAINT_TEST to signal that a test has been run.
>>      This will be printed as 'N' (originally for kuNit, as every other
>>      sensible letter was taken.)
>>          This should discourage people from running these tests on production
>>      systems, and to make it easier to tell if tests have been run
>>      accidentally (by loading the wrong configuration, etc.)
>>          Acked-by: Luis Chamberlain <mcgrof@kernel.org>
>>      Reviewed-by: Brendan Higgins <brendanhiggins@google.com>
>>      Signed-off-by: David Gow <davidgow@google.com>
>>      Signed-off-by: Shuah Khan <skhan@linuxfoundation.org>
>>
>>
>> The same situation, it's a special TAINT_TEST, and set for a good
>> reason. And there is also a case of TAINT_CRAP for staging.
> 
> TAINT is fine, I don't care about that. So we can certainly do that. My

Good you changed your mind

> main objection is just to gating it on lockdep/kasan or something like
> that.
> 

-- 
Pavel Begunkov