Re: RFC: chasing all idr_remove() misses

[Alexandre Ferrieux <alexandre.ferrieux@gmail.com>]

On 18/11/2024 12:33, Alexandre Ferrieux wrote:
> On 18/11/2024 03:47, Matthew Wilcox wrote:
>> 
>> IDR is deprecated.  I'm not excited about patches that make it better.
>> IDR users should be converted to use the XArray API.
> 
> Well, the problem is more about chasing a bad usage pattern than an API's
> limitations. Indeed, doing the same grep for Xarray shows that, out of 434
> (naked) call sites of xa_erase() in the kernel, 307 fail to check the result.
> 
> So, independently from the IDR->XArray transition (which may still take some
> time), it looks like there's a Damocles sword over a possibly huge number of
> areas, and that would handily be grabbed by a syzbot scan, with a ridiculous
> time investment: just add #ifdef ... WARN_ON_ONCE() ...#endif on the "return
> NULL" path of both idr_remove() and xa_erase().
> 
> Again, I'm not asking to do this on production kernels. All I'm asking is the
> detailed procedure to let the "established" fuzzers exercise that, as using
> syzkaller locally on my limited hardware is not an serious option.

Reading up on syzbot documentation, I can see that the recommended idiom for
such cases is #if IS_ENABLED(CONFIG_DEBUG_AID_FOR_SYZBOT):

>> https://github.com/google/syzkaller/blob/master/docs/syzbot.md says:
>
> However, syzbot kernel config always includes CONFIG_DEBUG_AID_FOR_SYZBOT=y
> setting, which is not normally present in kernel. What was used for
> particularly elusive bugs in the past is temporary merging some additional
> debugging code into linux-next under this config setting (e.g. more debug
> checks and/or debug output) and waiting for new crash reports from syzbot

So, if I submitted a patch doing WARN_ON_ONCE(..==NULL) under this flag in all
naked idr_remove() and xa_erase() call sites, with suitable macro decoration to
keep readability, would it stand a chance to be committed ?