drivers/bluetooth/btmtk.c | 12 ++++++------ drivers/bluetooth/btmtk.h | 1 + 2 files changed, 7 insertions(+), 6 deletions(-)
There may be a potential null pointer risk if offset value is
less than 0 when doing memcmp in btmtk_process_coredump().
Check offset is valid before doing memcmp.
Signed-off-by: Chris Lu <chris.lu@mediatek.com>
Co-developed-by: Sean Wang <sean.wang@mediatek.com>
Signed-off-by: Sean Wang <sean.wang@mediatek.com>
---
v2: fix typo
v3: fix bot checking error
v4: reduce variable 'offset' declaration in v3
---
drivers/bluetooth/btmtk.c | 12 ++++++------
drivers/bluetooth/btmtk.h | 1 +
2 files changed, 7 insertions(+), 6 deletions(-)
diff --git a/drivers/bluetooth/btmtk.c b/drivers/bluetooth/btmtk.c
index 786f775196ae..9cc789272ab7 100644
--- a/drivers/bluetooth/btmtk.c
+++ b/drivers/bluetooth/btmtk.c
@@ -395,12 +395,12 @@ int btmtk_process_coredump(struct hci_dev *hdev, struct sk_buff *skb)
/* Mediatek coredump data would be more than MTK_COREDUMP_NUM */
if (data->cd_info.cnt > MTK_COREDUMP_NUM &&
- skb->len > sizeof(MTK_COREDUMP_END) &&
- !memcmp((char *)&skb->data[skb->len - sizeof(MTK_COREDUMP_END)],
- MTK_COREDUMP_END, sizeof(MTK_COREDUMP_END) - 1)) {
- bt_dev_info(hdev, "Mediatek coredump end");
- hci_devcd_complete(hdev);
- }
+ skb->len > MTK_COREDUMP_END_LEN)
+ if (!memcmp((char *)&skb->data[skb->len - MTK_COREDUMP_END_LEN],
+ MTK_COREDUMP_END, MTK_COREDUMP_END_LEN - 1)) {
+ bt_dev_info(hdev, "Mediatek coredump end");
+ hci_devcd_complete(hdev);
+ }
break;
}
diff --git a/drivers/bluetooth/btmtk.h b/drivers/bluetooth/btmtk.h
index 68309dfe076a..56f5502baadf 100644
--- a/drivers/bluetooth/btmtk.h
+++ b/drivers/bluetooth/btmtk.h
@@ -24,6 +24,7 @@
#define MTK_COREDUMP_SIZE (1024 * 1000)
#define MTK_COREDUMP_END "coredump end"
+#define MTK_COREDUMP_END_LEN (sizeof(MTK_COREDUMP_END))
#define MTK_COREDUMP_NUM 255
enum {
--
2.18.0
Hi Chris, On Wed, Jul 12, 2023 at 6:03 AM Chris Lu <chris.lu@mediatek.com> wrote: > > There may be a potential null pointer risk if offset value is > less than 0 when doing memcmp in btmtk_process_coredump(). > Check offset is valid before doing memcmp. > > Signed-off-by: Chris Lu <chris.lu@mediatek.com> > Co-developed-by: Sean Wang <sean.wang@mediatek.com> > Signed-off-by: Sean Wang <sean.wang@mediatek.com> You should probably include the Fixes tag of the patch that introduced the problem. > --- > v2: fix typo > v3: fix bot checking error > v4: reduce variable 'offset' declaration in v3 > --- > drivers/bluetooth/btmtk.c | 12 ++++++------ > drivers/bluetooth/btmtk.h | 1 + > 2 files changed, 7 insertions(+), 6 deletions(-) > > diff --git a/drivers/bluetooth/btmtk.c b/drivers/bluetooth/btmtk.c > index 786f775196ae..9cc789272ab7 100644 > --- a/drivers/bluetooth/btmtk.c > +++ b/drivers/bluetooth/btmtk.c > @@ -395,12 +395,12 @@ int btmtk_process_coredump(struct hci_dev *hdev, struct sk_buff *skb) > > /* Mediatek coredump data would be more than MTK_COREDUMP_NUM */ > if (data->cd_info.cnt > MTK_COREDUMP_NUM && > - skb->len > sizeof(MTK_COREDUMP_END) && > - !memcmp((char *)&skb->data[skb->len - sizeof(MTK_COREDUMP_END)], > - MTK_COREDUMP_END, sizeof(MTK_COREDUMP_END) - 1)) { > - bt_dev_info(hdev, "Mediatek coredump end"); > - hci_devcd_complete(hdev); > - } > + skb->len > MTK_COREDUMP_END_LEN) > + if (!memcmp((char *)&skb->data[skb->len - MTK_COREDUMP_END_LEN], > + MTK_COREDUMP_END, MTK_COREDUMP_END_LEN - 1)) { > + bt_dev_info(hdev, "Mediatek coredump end"); > + hci_devcd_complete(hdev); > + } > > break; > } > diff --git a/drivers/bluetooth/btmtk.h b/drivers/bluetooth/btmtk.h > index 68309dfe076a..56f5502baadf 100644 > --- a/drivers/bluetooth/btmtk.h > +++ b/drivers/bluetooth/btmtk.h > @@ -24,6 +24,7 @@ > > #define MTK_COREDUMP_SIZE (1024 * 1000) > #define MTK_COREDUMP_END "coredump end" > +#define MTK_COREDUMP_END_LEN (sizeof(MTK_COREDUMP_END)) > #define MTK_COREDUMP_NUM 255 > > enum { > -- > 2.18.0 > -- Luiz Augusto von Dentz
On Wed, 2023-07-12 at 13:29 -0700, Luiz Augusto von Dentz wrote: > > External email : Please do not click links or open attachments until > you have verified the sender or the content. > Hi Chris, > > On Wed, Jul 12, 2023 at 6:03 AM Chris Lu <chris.lu@mediatek.com> > wrote: > > > > There may be a potential null pointer risk if offset value is > > less than 0 when doing memcmp in btmtk_process_coredump(). > > Check offset is valid before doing memcmp. > > > > Signed-off-by: Chris Lu <chris.lu@mediatek.com> > > Co-developed-by: Sean Wang <sean.wang@mediatek.com> > > Signed-off-by: Sean Wang <sean.wang@mediatek.com> > > You should probably include the Fixes tag of the patch that > introduced > the problem. > Hi Luiz, Thanks for reminding, I'll update Bug ad fixs info in commit message and send the v5 patch. BRs, Chris > > --- > > v2: fix typo > > v3: fix bot checking error > > v4: reduce variable 'offset' declaration in v3 > > --- > > drivers/bluetooth/btmtk.c | 12 ++++++------ > > drivers/bluetooth/btmtk.h | 1 + > > 2 files changed, 7 insertions(+), 6 deletions(-) > > > > diff --git a/drivers/bluetooth/btmtk.c b/drivers/bluetooth/btmtk.c > > index 786f775196ae..9cc789272ab7 100644 > > --- a/drivers/bluetooth/btmtk.c > > +++ b/drivers/bluetooth/btmtk.c > > @@ -395,12 +395,12 @@ int btmtk_process_coredump(struct hci_dev > *hdev, struct sk_buff *skb) > > > > /* Mediatek coredump data would be more than > MTK_COREDUMP_NUM */ > > if (data->cd_info.cnt > MTK_COREDUMP_NUM && > > - skb->len > sizeof(MTK_COREDUMP_END) && > > - !memcmp((char *)&skb->data[skb->len - > sizeof(MTK_COREDUMP_END)], > > - MTK_COREDUMP_END, > sizeof(MTK_COREDUMP_END) - 1)) { > > - bt_dev_info(hdev, "Mediatek coredump end"); > > - hci_devcd_complete(hdev); > > - } > > + skb->len > MTK_COREDUMP_END_LEN) > > + if (!memcmp((char *)&skb->data[skb->len - > MTK_COREDUMP_END_LEN], > > + MTK_COREDUMP_END, MTK_COREDUMP_END_LEN > - 1)) { > > + bt_dev_info(hdev, "Mediatek > coredump end"); > > + hci_devcd_complete(hdev); > > + } > > > > break; > > } > > diff --git a/drivers/bluetooth/btmtk.h b/drivers/bluetooth/btmtk.h > > index 68309dfe076a..56f5502baadf 100644 > > --- a/drivers/bluetooth/btmtk.h > > +++ b/drivers/bluetooth/btmtk.h > > @@ -24,6 +24,7 @@ > > > > #define MTK_COREDUMP_SIZE (1024 * 1000) > > #define MTK_COREDUMP_END "coredump end" > > +#define MTK_COREDUMP_END_LEN (sizeof(MTK_COREDUMP_END)) > > #define MTK_COREDUMP_NUM 255 > > > > enum { > > -- > > 2.18.0 > > > > > -- > Luiz Augusto von Dentz
© 2016 - 2024 Red Hat, Inc.