Received-SPF: pass (google.com: domain of linux-bluetooth+bounces-987-linux.lists.archive=gmail.com@vger.kernel.org designates 2604:1380:45e3:2400::1 as permitted sender) client-ip=2604:1380:45e3:2400::1;
Message-ID: <a35df10f-fd93-42f5-911e-26b015f8593a@quicinc.com>
Date: Tue, 9 Jan 2024 18:39:49 +0800
Precedence: bulk
MIME-Version: 1.0
User-Agent: Mozilla Thunderbird
Subject: Re: [PATCH v1 1/1] Bluetooth: hci_event: Fix wakeup BD_ADDR are
 wrongly recorded
To: Paul Menzel <pmenzel@molgen.mpg.de>
CC: <luiz.dentz@gmail.com>, <marcel@holtmann.org>, <johan.hedberg@gmail.com>,
        <linux-bluetooth@vger.kernel.org>, <stable@vger.kernel.org>
References: <1704789450-17754-1-git-send-email-quic_zijuhu@quicinc.com>
 <a4ec8dd2-22aa-4337-b9f4-b35563aa404f@molgen.mpg.de>
Content-Language: en-US
From: quic_zijuhu <quic_zijuhu@quicinc.com>
In-Reply-To: <a4ec8dd2-22aa-4337-b9f4-b35563aa404f@molgen.mpg.de>
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: 8bit

On 1/9/2024 5:58 PM, Paul Menzel wrote:
> Dear Zijun,
> 
> 
> Thank you very much for your patch. Should you resent, some nits for the commit message. For the summary, I suggest:
> 
> Bluetooth: hci_event: Fix wrongly recorded wakeup BD_ADDR
> 
will take your suggestions and send v2 patch
> Am 09.01.24 um 09:37 schrieb Zijun Hu:
>> hci_store_wake_reason() wrongly parses event HCI_Connection_Request
>> as HCI_Connection_Complete and HCI_Connection_Complete as
>> HCI_Connection_Request, so causes recording wakeup BD_ADDR error and
>> stability issue, it is fixed by this change.
> 
> Maybe: … stability issue. Fix it by using the correct field.
> will correct commit messages based on your suggestions.
> How did you reproduce the stability issues?
> 
> As you sent it to stable@vger.kernel.org, could you please add a Fixes: tag?
> 
i will take it as potential stability issue since it maybe access unexpected memory area.
don't send it to  stable@vger.kernel.org any more.

actually. i just read code and find this issue.

>> Signed-off-by: Zijun Hu <quic_zijuhu@quicinc.com>
>> ---
>>   net/bluetooth/hci_event.c | 4 ++--
>>   1 file changed, 2 insertions(+), 2 deletions(-)
>>
>> diff --git a/net/bluetooth/hci_event.c b/net/bluetooth/hci_event.c
>> index ef8c3bed7361..22b22c264c2a 100644
>> --- a/net/bluetooth/hci_event.c
>> +++ b/net/bluetooth/hci_event.c
>> @@ -7420,10 +7420,10 @@ static void hci_store_wake_reason(struct hci_dev *hdev, u8 event,
>>        * keep track of the bdaddr of the connection event that woke us up.
>>        */
>>       if (event == HCI_EV_CONN_REQUEST) {
>> -        bacpy(&hdev->wake_addr, &conn_complete->bdaddr);
>> +        bacpy(&hdev->wake_addr, &conn_request->bdaddr);
>>           hdev->wake_addr_type = BDADDR_BREDR;
>>       } else if (event == HCI_EV_CONN_COMPLETE) {
>> -        bacpy(&hdev->wake_addr, &conn_request->bdaddr);
>> +        bacpy(&hdev->wake_addr, &conn_complete->bdaddr);
>>           hdev->wake_addr_type = BDADDR_BREDR;
>>       } else if (event == HCI_EV_LE_META) {
>>           struct hci_ev_le_meta *le_ev = (void *)skb->data;
> 
> Reviewed-by: Paul Menzel <pmenzel@molgen.mpg.de>
> 
> 
> Kind regards,
> 
> Paul