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

[Jens Axboe <axboe@kernel.dk>]

On 5/30/25 12:14 PM, Pavel Begunkov wrote:
> 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

Yes, my main objection was (and is) having nonsensical dependencies.

-- 
Jens Axboe