On 4/17/2024 2:30 PM, Johan Hovold wrote:
> On Wed, Apr 17, 2024 at 11:49:52AM +0800, Zijun Hu wrote:
>> hu->serdev is nullptr and will cause nullptr dereference if qca_setup()
>> is called by non-serdev device, fixed by null check before access.
>
> No, this patch is not correct.
>
i don't think so, nullptr checking for hu->serdev has been performed within qca_setup()
everywhere when need to access serdev related members since this function will be called
by both serdev and none-serdev. so suggest add such checking.
>> Fixes: 77f45cca8bc5 ("Bluetooth: qca: fix device-address endianness")
>> Signed-off-by: Zijun Hu <quic_zijuhu@xxxxxxxxxxx>
>> ---
>> drivers/bluetooth/hci_qca.c | 9 +++++----
>> 1 file changed, 5 insertions(+), 4 deletions(-)
>>
>> diff --git a/drivers/bluetooth/hci_qca.c b/drivers/bluetooth/hci_qca.c
>> index 92fa20f5ac7d..9c6573c727e1 100644
>> --- a/drivers/bluetooth/hci_qca.c
>> +++ b/drivers/bluetooth/hci_qca.c
>> @@ -1905,10 +1905,11 @@ static int qca_setup(struct hci_uart *hu)
>> case QCA_WCN6750:
>> case QCA_WCN6855:
>> case QCA_WCN7850:
>> - qcadev = serdev_device_get_drvdata(hu->serdev);
>
> Non-serdev controllers have type QCA_ROME (see qca_soc_type()) so will
> never end up in this path.
>
i have submitted below patches to add supports for all other non-serdev controllers.
https://patchwork.kernel.org/project/bluetooth/list/?series=844120
> I verified this when I wrote the patch and also fixed up a couple of
> real non-serdev bugs here:
>
> https://lore.kernel.org/lkml/20240319154611.2492-1-johan+linaro@xxxxxxxxxx/
>
actually, i have submitted below fix for this issue earlier.
https://lore.kernel.org/all/1704960978-5437-1-git-send-email-quic_zijuhu@xxxxxxxxxxx/
>> - if (qcadev->bdaddr_property_broken)
>> - set_bit(HCI_QUIRK_BDADDR_PROPERTY_BROKEN, &hdev->quirks);
>> -
>> + if (hu->serdev) {
>> + qcadev = serdev_device_get_drvdata(hu->serdev);
>> + if (qcadev->bdaddr_property_broken)
>> + set_bit(HCI_QUIRK_BDADDR_PROPERTY_BROKEN, &hdev->quirks);
>> + }
>> hci_set_aosp_capable(hdev);
>>
>> ret = qca_read_soc_version(hdev, &ver, soc_type);
>
> Johan
