From: Jia-Ju Bai <[email protected]>
Date: Sat, 4 May 2019 11:08:13 +0800
> When platform_driver_register() fails, pci_unregister_driver() is not
> called to release the resource allocated by pci_register_driver().
>
> To fix this bug, error handling code for platform_driver_register() and
> pci_register_driver() is separately implemented.
>
> This bug is found by a runtime fuzzing tool named FIZZER written by us.
>
> Signed-off-by: Jia-Ju Bai <[email protected]>
I think the idea here is that PCI is not enabled in the kernel, it is
fine for the pci register to fail and only the platform register to
succeed.
You are breaking that.
On 2019/5/4 11:13, David Miller wrote:
> From: Jia-Ju Bai <[email protected]>
> Date: Sat, 4 May 2019 11:08:13 +0800
>
>> When platform_driver_register() fails, pci_unregister_driver() is not
>> called to release the resource allocated by pci_register_driver().
>>
>> To fix this bug, error handling code for platform_driver_register() and
>> pci_register_driver() is separately implemented.
>>
>> This bug is found by a runtime fuzzing tool named FIZZER written by us.
>>
>> Signed-off-by: Jia-Ju Bai <[email protected]>
> I think the idea here is that PCI is not enabled in the kernel, it is
> fine for the pci register to fail and only the platform register to
> succeed.
>
> You are breaking that.
Okay, I can understand it.
If so, I think that platform_driver_register() should be called before
pci_register_driver(), and it is still necessary to separately handle
their errors.
If you agree, I will send a v2 patch.
Best wishes,
Jia-Ju Bai
From: Jia-Ju Bai <[email protected]>
Date: Sat, 4 May 2019 11:23:06 +0800
>
>
> On 2019/5/4 11:13, David Miller wrote:
>> From: Jia-Ju Bai <[email protected]>
>> Date: Sat, 4 May 2019 11:08:13 +0800
>>
>>> When platform_driver_register() fails, pci_unregister_driver() is not
>>> called to release the resource allocated by pci_register_driver().
>>>
>>> To fix this bug, error handling code for platform_driver_register()
>>> and
>>> pci_register_driver() is separately implemented.
>>>
>>> This bug is found by a runtime fuzzing tool named FIZZER written by
>>> us.
>>>
>>> Signed-off-by: Jia-Ju Bai <[email protected]>
>> I think the idea here is that PCI is not enabled in the kernel, it is
>> fine for the pci register to fail and only the platform register to
>> succeed.
>>
>> You are breaking that.
>
> Okay, I can understand it.
> If so, I think that platform_driver_register() should be called before
> pci_register_driver(), and it is still necessary to separately handle
> their errors.
> If you agree, I will send a v2 patch.
It is only a failure if both fail.
If at least one succeeds, the driver can potentially probe properly.
On 2019/5/4 11:41, David Miller wrote:
> From: Jia-Ju Bai <[email protected]>
> Date: Sat, 4 May 2019 11:23:06 +0800
>
>>
>> On 2019/5/4 11:13, David Miller wrote:
>>> From: Jia-Ju Bai <[email protected]>
>>> Date: Sat, 4 May 2019 11:08:13 +0800
>>>
>>>> When platform_driver_register() fails, pci_unregister_driver() is not
>>>> called to release the resource allocated by pci_register_driver().
>>>>
>>>> To fix this bug, error handling code for platform_driver_register()
>>>> and
>>>> pci_register_driver() is separately implemented.
>>>>
>>>> This bug is found by a runtime fuzzing tool named FIZZER written by
>>>> us.
>>>>
>>>> Signed-off-by: Jia-Ju Bai <[email protected]>
>>> I think the idea here is that PCI is not enabled in the kernel, it is
>>> fine for the pci register to fail and only the platform register to
>>> succeed.
>>>
>>> You are breaking that.
>> Okay, I can understand it.
>> If so, I think that platform_driver_register() should be called before
>> pci_register_driver(), and it is still necessary to separately handle
>> their errors.
>> If you agree, I will send a v2 patch.
> It is only a failure if both fail.
>
> If at least one succeeds, the driver can potentially probe properly.
Thanks for the explanation, I understand now :)
The code logic seems a little strange, but it should be correct in
rhine_init()...
If so, if one fails, I wonder whether it is correct for rhine_cleanup()
to call both platform_driver_unregister() and pci_unregister_driver()?
Best wishes,
Jia-Ju Bai