Re: [syzbot] [iommu?] memory leak in iommu_fwspec_init

[Krystian Kaniewski <krystianmkaniewski@gmail.com>]

On 11/6/26 16:08, Krystian Kaniewski wrote:
>> This report is generated by a bot. It may contain errors.
>> See https://goo.gl/tpsmEJ for more information about syzbot.
>> syzbot engineers can be reached at syzkaller@googlegroups.com.
>>
>> syzbot will keep track of this issue. See:
>> https://goo.gl/tpsmEJ#status for how to communicate with syzbot.
>>
>> If the report is already addressed, let syzbot know by replying with:
>> #syz fix: exact-commit-title
>>
>> If you want syzbot to run the reproducer, reply with:
>> #syz test: git://repo/address.git branch-or-commit-hash
>> If you attach or paste a git patch, syzbot will apply it before testing.
>>
>> If you want to overwrite report's subsystems, reply with:
>> #syz set subsystems: new-subsystem
>> (See the list of subsystem names on the web dashboard)
>>
>> If the report is a duplicate of another one, reply with:
>> #syz dup: exact-subject-of-another-report
>>
>> If you want to undo deduplication, reply with:
>> #syz undup
> Please move the fix out of iommu_fwspec_init() and into
> iommu_mock_device_add(), and leave iommu_fwspec_init() unchanged. The
> same core-level approach for this leak was already rejected by Robin
> Murphy on 2026-11-02 [1]. iommu_fwspec_init() is designed to run
> together with iommu_probe_device() or with a caller that cleans up after
> a failure, as of_iommu_configure() does. The defect is the early return
> in iommu_mock_device_add().
> 
> The leak happens because iommu_mock_device_add() returns the error of
> iommu_fwspec_init() without freeing dev->iommu. iommu_fwspec_init() can
> allocate dev->iommu with dev_iommu_get() and then fail to allocate the
> fwspec. The mock device has not been added at that point, so no bus
> notifier will ever call iommu_release_device() for it, and the
> put_device() in mock_dev_create() frees only the mock device. When
> iommu_fwspec_init() fails, iommu_mock_device_add() should call
> dev_iommu_free() if dev->iommu is set. The check is needed because
> iommu_fwspec_init() can also fail before it allocates anything, for
> example when dev_iommu_get() itself fails. The existing dev_iommu_free()
> on the device_add() failure path is correct and must stay. One error
> label shared by both failure paths is a clean way to write it.
> 
> Please change the tag to:
> 
>   Fixes: 2a918911ed3d ("iommufd: Register iommufd mock devices with fwspec")
> 
> That commit added the early return that leaks. The current tag names
> 72acd9df18f1, a v5.7 change, so stable tooling would try to carry a core
> change to trees that do not even have the mock helper, which only exists
> since v6.18.
> 
> The commit message has to describe the new placement: name the early
> return in iommu_mock_device_add() as the cause and explain why nothing
> else frees dev->iommu for a mock device that was never added.
> 
> While rewriting the message, please also keep it short. From the
> kmemleak report, the allocation chain (dev_iommu_get, iommu_fwspec_init
> and iommu_mock_device_add with their source lines) is enough. The hex
> dump, the object address, the comm line and the syscall entry frames can
> go. Two or three short paragraphs are enough for this change.
> 
> [1] https://lore.kernel.org/all/7e209fb2-4bda-4207-9109-c67c8df41e05@arm.com/
> 
Please disregard the previous mail in this thread. It was meant as
review feedback on syzbot's AI-generated patch for this report, which is
still in moderation and was not posted to this list. It does not refer
to Qing Zhang's patch, which already makes this fix in
iommu_mock_device_add(). Sorry for the noise.
-- 
Krystian Kaniewski