Received-SPF: pass (google.com: domain of linux-bluetooth-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
MIME-Version: 1.0
References: <1681213778-31754-1-git-send-email-quic_zijuhu@quicinc.com>
In-Reply-To: <1681213778-31754-1-git-send-email-quic_zijuhu@quicinc.com>
From:   Luiz Augusto von Dentz <luiz.dentz@gmail.com>
Date:   Fri, 14 Apr 2023 14:03:17 -0700
Message-ID: <CABBYNZLJGmSaL1J0VSRU7vhTU-gyeqMndWxgwS2=d4-Bywfjxg@mail.gmail.com>
Subject: Re: [PATCH v1] Bluetooth: Optimize devcoredump API hci_devcd_init()
To:     Zijun Hu <quic_zijuhu@quicinc.com>
Cc:     marcel@holtmann.org, johan.hedberg@gmail.com, davem@davemloft.net,
        edumazet@google.com, kuba@kernel.org, pabeni@redhat.com,
        linux-kernel@vger.kernel.org, linux-bluetooth@vger.kernel.org,
        netdev@vger.kernel.org, abhishekpandit@chromium.org
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: quoted-printable
Precedence: bulk

Hi Zijun,

On Tue, Apr 11, 2023 at 4:49=E2=80=AFAM Zijun Hu <quic_zijuhu@quicinc.com> =
wrote:
>
> API hci_devcd_init() stores u32 type to memory without specific byte
> order, let us store with little endian in order to be loaded and
> parsed by devcoredump core rightly.

This looks like a fix if devcoredump expects little endian, so I'd
suggest rephrasing to state it in the subject line, also add the Fixes
tag for the commit that introduces this problem.

> Signed-off-by: Zijun Hu <quic_zijuhu@quicinc.com>
> ---
>  net/bluetooth/coredump.c | 13 +++++++------
>  1 file changed, 7 insertions(+), 6 deletions(-)
>
> diff --git a/net/bluetooth/coredump.c b/net/bluetooth/coredump.c
> index 08fa98505454..d2d2624ec708 100644
> --- a/net/bluetooth/coredump.c
> +++ b/net/bluetooth/coredump.c
> @@ -5,6 +5,7 @@
>
>  #include <linux/devcoredump.h>
>
> +#include <asm/unaligned.h>
>  #include <net/bluetooth/bluetooth.h>
>  #include <net/bluetooth/hci_core.h>
>
> @@ -180,25 +181,25 @@ static int hci_devcd_prepare(struct hci_dev *hdev, =
u32 dump_size)
>
>  static void hci_devcd_handle_pkt_init(struct hci_dev *hdev, struct sk_bu=
ff *skb)
>  {
> -       u32 *dump_size;
> +       u32 dump_size;
>
>         if (hdev->dump.state !=3D HCI_DEVCOREDUMP_IDLE) {
>                 DBG_UNEXPECTED_STATE();
>                 return;
>         }
>
> -       if (skb->len !=3D sizeof(*dump_size)) {
> +       if (skb->len !=3D sizeof(dump_size)) {
>                 bt_dev_dbg(hdev, "Invalid dump init pkt");
>                 return;
>         }
>
> -       dump_size =3D skb_pull_data(skb, sizeof(*dump_size));
> -       if (!*dump_size) {
> +       dump_size =3D get_unaligned_le32(skb_pull_data(skb, 4));
> +       if (!dump_size) {
>                 bt_dev_err(hdev, "Zero size dump init pkt");
>                 return;
>         }
>
> -       if (hci_devcd_prepare(hdev, *dump_size)) {
> +       if (hci_devcd_prepare(hdev, dump_size)) {
>                 bt_dev_err(hdev, "Failed to prepare for dump");
>                 return;
>         }
> @@ -441,7 +442,7 @@ int hci_devcd_init(struct hci_dev *hdev, u32 dump_siz=
e)
>                 return -ENOMEM;
>
>         hci_dmp_cb(skb)->pkt_type =3D HCI_DEVCOREDUMP_PKT_INIT;
> -       skb_put_data(skb, &dump_size, sizeof(dump_size));
> +       put_unaligned_le32(dump_size, skb_put(skb, 4));
>
>         skb_queue_tail(&hdev->dump.dump_q, skb);
>         queue_work(hdev->workqueue, &hdev->dump.dump_rx);
> --
> The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum=
, a Linux Foundation Collaborative Project
>


--=20
Luiz Augusto von Dentz