Hi Marcel,
Friendly reminder.
> -----Original Message-----
> From: K, Kiran
> Sent: Thursday, October 7, 2021 11:36 AM
> To: Luiz Augusto von Dentz <luiz.dentz@xxxxxxxxx>; Marcel Holtmann
> <marcel@xxxxxxxxxxxx>
> Cc: 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, 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
Thanks,
Kiran
