Matthew Wilcox wrote:
> On Fri, Nov 29, 2024 at 12:30:35AM +0100, Jann Horn wrote:
> > > ------------[ cut here ]------------
> > > WARNING: CPU: 0 PID: 16 at io_uring/tctx.c:51 __io_uring_free+0xfa/0x140 io_uring/tctx.c:51
> >
> > This warning is a check for WARN_ON_ONCE(!xa_empty(&tctx->xa)); and as
> > Jens pointed out, this was triggered after error injection caused a
> > memory allocation inside xa_store() to fail.
> >
> > Is there maybe an issue where xa_store() can fail midway through while
> > allocating memory for the xarray, so that xa_empty() is no longer true
> > even though there is nothing in the xarray? (And if yes, is that
> > working as intended?)
>
> Yes, that's a known possibility. We have similar problems when people
> use error injection with mapping->i_pages. The effort to fix it seems
> disproportionate to the severity of the problem.
Found this discussion while investigating memory leak in radix_tree_insert [1].
That report has a similar cause - a fault injection in the innards of
radix_tree (say, xarray) allocating loop, then the absence of release of
already allocated internal xarray memory afterall.
I wonder whether just the plain usage of xa_destroy() should be considered
a fix for these kinds of failures. Are there any pitfalls? xa_destroy() is
claimed to cleanup the internal xarray memory.
Judging by ca6484cd308a ("io_uring: no need to call xa_destroy() on empty
xarray"), seems some pitfalls do exist but still..
Would be glad to have a look into the previous discussions of this problem
if they exist - in case I'm raising the questions that were already
answered. Thanks!
P.S. there is no variant of xa_destroy() for radix tree. I think nobody
noticed this since it may have an effect only on these types of bugs
triggered by fault injection. If you think adding it overall makes sense
then I'd try to prepare a patch.
[1]: https://syzkaller.appspot.com/bug?extid=006987d1be3586e13555
