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 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.

-- 
Pavel Begunkov