On 3/12/24 8:23 AM, Fedor Pchelkin wrote:
No upstream commit exists for this patch.
Backport of commit 705318a99a13 ("io_uring/af_unix: disable sending io_uring over sockets") introduced registered files leaks in 5.10/5.15 stable branches when CONFIG_UNIX is enabled.
The 5.10/5.15 backports removed io_sqe_file_register() calls from io_install_fixed_file() and __io_sqe_files_update() so that newly added files aren't passed to UNIX-related skbs and thus can't be put during unregistering process. Skbs in the ring socket receive queue are released but there is no skb having reference to the newly updated file.
In other words, when CONFIG_UNIX is enabled there would be no fput() when files are unregistered for the corresponding fget() from io_install_fixed_file() and __io_sqe_files_update().
Drop several code paths related to SCM_RIGHTS as a partial change from commit 6e5e6d274956 ("io_uring: drop any code related to SCM_RIGHTS"). This code is useless in stable branches now, too, but is causing leaks in 5.10/5.15.
As stated above, the affected code was removed in upstream by commit 6e5e6d274956 ("io_uring: drop any code related to SCM_RIGHTS").
Fresher stables from 6.1 have io_file_need_scm() stub function which usage is effectively equivalent to dropping most of SCM-related code.
5.4 seems not to be affected with this problem since SCM-related functions have been dropped there by the backport-patch.
Found by Linux Verification Center (linuxtesting.org) with Syzkaller.
Fixes: 705318a99a13 ("io_uring/af_unix: disable sending io_uring over sockets") Signed-off-by: Fedor Pchelkin pchelkin@ispras.ru
I feel io_uring-SCM related code should be dropped entirely from the stable branches as the backports already differ greatly between versions and some parts are still kept, some have been dropped in a non-consistent order. Though this might contradict with stable kernel rules or be inappropriate for some other reason.
Looks fine to me, and I agree, it makes much more sense to drop it all from 5.10/5.15-stable as well to keep them in sync with upstream. And I think this is fine for stable, dropping code is always a good thing.