Hi Marcel,
On 06/16/2015 11:29 PM, Marcel Holtmann wrote:
Hi Chan-yeol,
If h4_recv() return ERR_PTR instead sk_buff pointer, it should be
cleared once dereference is completed for the further reference such as
h4_recv(), or h4_close().
I have no idea what the h4_close has to do with it? Can you explain.
If h4->rx_skb has ERR_PTR , kfree_skb() would dereference of ERR_PTR.
This is easily reproduced on my board when I turn off BT.
Signed-off-by: Chan-yeol Park <chanyeol.park@xxxxxxxxxxx>
---
drivers/bluetooth/hci_h4.c | 2 ++
1 file changed, 2 insertions(+)
diff --git a/drivers/bluetooth/hci_h4.c b/drivers/bluetooth/hci_h4.c
index f7190f0..a8acd99 100644
--- a/drivers/bluetooth/hci_h4.c
+++ b/drivers/bluetooth/hci_h4.c
@@ -133,6 +133,7 @@ static int h4_recv(struct hci_uart *hu, const void *data, int count)
if (IS_ERR(h4->rx_skb)) {
int err = PTR_ERR(h4->rx_skb);
BT_ERR("%s: Frame reassembly failed (%d)", hu->hdev->name, err);
+ h4->rx_skb = NULL;
return err;
}
Isn't this better fixed in h4_recv_buf directly:
Yes it's better. I think if we use ERR_PTR this would be right.
@@ -173,7 +173,7 @@ struct sk_buff *h4_recv_buf(struct hci_dev *hdev, struct sk_buff *skb,
while (count) {
int i, len;
- if (!skb) {
+ if (IS_ERR_OR_NULL(skb)) {
for (i = 0; i < pkts_count; i++) {
if (buffer[0] != (&pkts[i])->type)
continue;
@@ -248,6 +249,7 @@ struct sk_buff *h4_recv_buf(struct hci_dev *hdev, struct sk_buff *skb,
break;
default:
/* Unsupported variable length */
+
This change seems totally unrelated.
Sorry, Unexpectedly this blank line is added.
kfree_skb(skb);
return ERR_PTR(-EILSEQ);
}
Regards
Marcel
I would raise v2.
Thanks
Chanyeol
--
To unsubscribe from this list: send the line "unsubscribe linux-bluetooth" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
