On 4/17/2024 3:10 PM, Johan Hovold wrote:
> [ Please wrap you mails at 72 columns or so and trim unnecessary context
> when replying. ]
>
> On Wed, Apr 17, 2024 at 02:51:38PM +0800, quic_zijuhu wrote:
>> 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.
>
> Your patch is not correct since you claim that this path can trigger a
> NULL pointer dereference. As I point out below that is currently not
> possible.
>
> If you need this for some future change you need to say so in the commit
> message and drop the bogus Fixes tag.
>
will add this change in to btattach support serials.
>>>> Fixes: 77f45cca8bc5 ("Bluetooth: qca: fix device-address endianness")
>>>> Signed-off-by: Zijun Hu <quic_zijuhu@xxxxxxxxxxx>
>
>>>> @@ -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
>
> Ok, you need it for some future changes, but I'm afraid that adding new
> random vendor specific ioctls like you do in that is series is a no-go.
>
it is a generic ioctl, for QCA, it is used to specific soc_type. it maybe be used
by other vendor driver to set user specified info.
> Why are you trying to revive the old line-discipline when we have
> serdev?
>
we usually need to use tool btattach which will generate non-serdev devices to attach a BT module to generic PC to
complete various development and verification, customer also have requirements
to use tool btattach as explained by below link
https://patchwork.kernel.org/project/bluetooth/patch/1712939188-25529-5-git-send-email-quic_zijuhu@xxxxxxxxxxx/
> In any case, a change like this one would should be included in that
> series so that it's clear that it is only needed for your proposed
> further changes.
> okay, it has been included in the updated patch serials, will update the serials to include change
in question.
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/
>
> Ok.
>
> Johan
