Hi Marcel, Luiz,
> -----Original Message-----
> From: Luiz Augusto von Dentz <luiz.dentz@xxxxxxxxx>
> Sent: Wednesday, October 6, 2021 11:19 PM
> To: Marcel Holtmann <marcel@xxxxxxxxxxxx>
> Cc: K, Kiran <kiran.k@xxxxxxxxx>; linux-bluetooth <linux-
> bluetooth@xxxxxxxxxxxxxxx>; Srivatsa, Ravishankar
> <ravishankar.srivatsa@xxxxxxxxx>; Tumkur Narayan, Chethan
> <chethan.tumkur.narayan@xxxxxxxxx>; An, Tedd <tedd.an@xxxxxxxxx>
> Subject: Re: [PATCH v1] Bluetooth: btintel: Fix bdaddress comparison with
> garbage value
>
> Hi Marcel,
>
> On Wed, Oct 6, 2021 at 1:52 AM Marcel Holtmann <marcel@xxxxxxxxxxxx>
> wrote:
> >
> > Hi Kiran,
> >
> > > Intel Read Verision(TLV) data is parsed into a local structure
> > > variable and it contains a field for bd address. Bd address is
> > > returned only in bootloader mode and hence bd address in TLV
> > > structure needs to be validated only if controller is present in boot loader
> mode.
> > >
> > > Signed-off-by: Kiran K <kiran.k@xxxxxxxxx>
> > > ---
> > > drivers/bluetooth/btintel.c | 19 +++++++++++--------
> > > 1 file changed, 11 insertions(+), 8 deletions(-)
> > >
> > > diff --git a/drivers/bluetooth/btintel.c
> > > b/drivers/bluetooth/btintel.c index 9359bff47296..d1703cc99705
> > > 100644
> > > --- a/drivers/bluetooth/btintel.c
> > > +++ b/drivers/bluetooth/btintel.c
> > > @@ -2081,14 +2081,16 @@ static int
> btintel_prepare_fw_download_tlv(struct hci_dev *hdev,
> > > if (ver->img_type == 0x03) {
> > > btintel_clear_flag(hdev, INTEL_BOOTLOADER);
> > > btintel_check_bdaddr(hdev);
> > > - }
> > > -
> > > - /* If the OTP has no valid Bluetooth device address, then there will
> > > - * also be no valid address for the operational firmware.
> > > - */
> > > - if (!bacmp(&ver->otp_bd_addr, BDADDR_ANY)) {
> > > - bt_dev_info(hdev, "No device address configured");
> > > - set_bit(HCI_QUIRK_INVALID_BDADDR, &hdev->quirks);
> > > + } else {
> > > + /*
> > > + * Check for valid bd address in boot loader mode. Device
> > > + * will be marked as unconfigured if empty bd address is
> > > + * found.
> > > + */
> > > + if (!bacmp(&ver->otp_bd_addr, BDADDR_ANY)) {
> > > + bt_dev_info(hdev, "No device address configured");
> > > + set_bit(HCI_QUIRK_INVALID_BDADDR, &hdev->quirks);
> > > + }
> > > }
> > >
> > > btintel_get_fw_name_tlv(ver, fwname, sizeof(fwname), "sfi");
> > > @@ -2466,6 +2468,7 @@ static int btintel_setup_combined(struct
> hci_dev *hdev)
> > > goto exit_error;
> > > }
> > >
> > > + memset(&ver_tlv, 0, sizeof(ver_tlv));
> >
> > this change is not described in the commit message. Why is that now out of
> a sudden needed?
>
> I guess this is just to make sure the ver_tlv is initialized so its otp_bd_addr be
> set to all zeros (BDADDR_ANY) otherwise the code above doesn't work as it
> attempts to compare to BDADDR_ANY.
Yes. If not memset, then garbage value is compared against BDADDR_ANY.
>
> > Regards
> >
> > Marcel
> >
>
>
> --
> Luiz Augusto von Dentz
