* [PATCH v3 1/3] Bluetooth: hci_sock: Replace use of memcpy_from_msg with bt_skb_sendmsg
@ 2021-09-15 23:01 Luiz Augusto von Dentz
2021-09-15 23:01 ` [PATCH v3 2/3] Bluetooth: Fix passing NULL to PTR_ERR Luiz Augusto von Dentz
2021-09-15 23:01 ` [PATCH v3 3/3] Bluetooth: SCO: Fix sco_send_frame returning skb->len Luiz Augusto von Dentz
0 siblings, 2 replies; 3+ messages in thread
From: Luiz Augusto von Dentz @ 2021-09-15 23:01 UTC (permalink / raw)
To: linux-bluetooth
From: Luiz Augusto von Dentz <luiz.von.dentz@intel.com>
This makes use of bt_skb_sendmsg instead of allocating a different
buffer to be used with memcpy_from_msg which cause one extra copy.
Signed-off-by: Luiz Augusto von Dentz <luiz.von.dentz@intel.com>
---
net/bluetooth/hci_sock.c | 100 +++++++++++++++------------------------
1 file changed, 37 insertions(+), 63 deletions(-)
diff --git a/net/bluetooth/hci_sock.c b/net/bluetooth/hci_sock.c
index e481eee8e61e..39309ef6af4a 100644
--- a/net/bluetooth/hci_sock.c
+++ b/net/bluetooth/hci_sock.c
@@ -1510,7 +1510,8 @@ static int hci_sock_recvmsg(struct socket *sock, struct msghdr *msg,
return err ? : copied;
}
-static int hci_mgmt_cmd(struct hci_mgmt_chan *chan, struct sock *sk, void *buf, size_t msglen)
+static int hci_mgmt_cmd(struct hci_mgmt_chan *chan, struct sock *sk,
+ struct sk_buff *skb)
{
u8 *cp;
struct mgmt_hdr *hdr;
@@ -1520,31 +1521,31 @@ static int hci_mgmt_cmd(struct hci_mgmt_chan *chan, struct sock *sk, void *buf,
bool var_len, no_hdev;
int err;
- BT_DBG("got %zu bytes", msglen);
+ BT_DBG("got %d bytes", skb->len);
- if (msglen < sizeof(*hdr))
+ if (skb->len < sizeof(*hdr))
return -EINVAL;
- hdr = buf;
+ hdr = (void *)skb->data;
opcode = __le16_to_cpu(hdr->opcode);
index = __le16_to_cpu(hdr->index);
len = __le16_to_cpu(hdr->len);
- if (len != msglen - sizeof(*hdr)) {
+ if (len != skb->len - sizeof(*hdr)) {
err = -EINVAL;
goto done;
}
if (chan->channel == HCI_CHANNEL_CONTROL) {
- struct sk_buff *skb;
+ struct sk_buff *cmd;
/* Send event to monitor */
- skb = create_monitor_ctrl_command(sk, index, opcode, len,
- buf + sizeof(*hdr));
- if (skb) {
- hci_send_to_channel(HCI_CHANNEL_MONITOR, skb,
+ cmd = create_monitor_ctrl_command(sk, index, opcode, len,
+ skb->data + sizeof(*hdr));
+ if (cmd) {
+ hci_send_to_channel(HCI_CHANNEL_MONITOR, cmd,
HCI_SOCK_TRUSTED, NULL);
- kfree_skb(skb);
+ kfree_skb(cmd);
}
}
@@ -1609,13 +1610,13 @@ static int hci_mgmt_cmd(struct hci_mgmt_chan *chan, struct sock *sk, void *buf,
if (hdev && chan->hdev_init)
chan->hdev_init(sk, hdev);
- cp = buf + sizeof(*hdr);
+ cp = skb->data + sizeof(*hdr);
err = handler->func(sk, hdev, cp, len);
if (err < 0)
goto done;
- err = msglen;
+ err = skb->len;
done:
if (hdev)
@@ -1624,10 +1625,10 @@ static int hci_mgmt_cmd(struct hci_mgmt_chan *chan, struct sock *sk, void *buf,
return err;
}
-static int hci_logging_frame(struct sock *sk, void *buf, int len, unsigned int flags)
+static int hci_logging_frame(struct sock *sk, struct sk_buff *skb,
+ unsigned int flags)
{
struct hci_mon_hdr *hdr;
- struct sk_buff *skb;
struct hci_dev *hdev;
u16 index;
int err;
@@ -1636,21 +1637,13 @@ static int hci_logging_frame(struct sock *sk, void *buf, int len, unsigned int f
* the priority byte, the ident length byte and at least one string
* terminator NUL byte. Anything shorter are invalid packets.
*/
- if (len < sizeof(*hdr) + 3)
+ if (skb->len < sizeof(*hdr) + 3)
return -EINVAL;
- skb = bt_skb_send_alloc(sk, len, flags & MSG_DONTWAIT, &err);
- if (!skb)
- return err;
-
- memcpy(skb_put(skb, len), buf, len);
-
hdr = (void *)skb->data;
- if (__le16_to_cpu(hdr->len) != len - sizeof(*hdr)) {
- err = -EINVAL;
- goto drop;
- }
+ if (__le16_to_cpu(hdr->len) != skb->len - sizeof(*hdr))
+ return -EINVAL;
if (__le16_to_cpu(hdr->opcode) == 0x0000) {
__u8 priority = skb->data[sizeof(*hdr)];
@@ -1669,25 +1662,20 @@ static int hci_logging_frame(struct sock *sk, void *buf, int len, unsigned int f
* The message follows the ident string (if present) and
* must be NUL terminated. Otherwise it is not a valid packet.
*/
- if (priority > 7 || skb->data[len - 1] != 0x00 ||
- ident_len > len - sizeof(*hdr) - 3 ||
- skb->data[sizeof(*hdr) + ident_len + 1] != 0x00) {
- err = -EINVAL;
- goto drop;
- }
+ if (priority > 7 || skb->data[skb->len - 1] != 0x00 ||
+ ident_len > skb->len - sizeof(*hdr) - 3 ||
+ skb->data[sizeof(*hdr) + ident_len + 1] != 0x00)
+ return -EINVAL;
} else {
- err = -EINVAL;
- goto drop;
+ return -EINVAL;
}
index = __le16_to_cpu(hdr->index);
if (index != MGMT_INDEX_NONE) {
hdev = hci_dev_get(index);
- if (!hdev) {
- err = -ENODEV;
- goto drop;
- }
+ if (!hdev)
+ return -ENODEV;
} else {
hdev = NULL;
}
@@ -1695,13 +1683,11 @@ static int hci_logging_frame(struct sock *sk, void *buf, int len, unsigned int f
hdr->opcode = cpu_to_le16(HCI_MON_USER_LOGGING);
hci_send_to_channel(HCI_CHANNEL_MONITOR, skb, HCI_SOCK_TRUSTED, NULL);
- err = len;
+ err = skb->len;
if (hdev)
hci_dev_put(hdev);
-drop:
- kfree_skb(skb);
return err;
}
@@ -1713,7 +1699,6 @@ static int hci_sock_sendmsg(struct socket *sock, struct msghdr *msg,
struct hci_dev *hdev;
struct sk_buff *skb;
int err;
- void *buf;
const unsigned int flags = msg->msg_flags;
BT_DBG("sock %p sk %p", sock, sk);
@@ -1727,13 +1712,9 @@ static int hci_sock_sendmsg(struct socket *sock, struct msghdr *msg,
if (len < 4 || len > hci_pi(sk)->mtu)
return -EINVAL;
- buf = kmalloc(len, GFP_KERNEL);
- if (!buf)
- return -ENOMEM;
- if (memcpy_from_msg(buf, msg, len)) {
- kfree(buf);
- return -EFAULT;
- }
+ skb = bt_skb_sendmsg(sk, msg, len, len, 0, 0);
+ if (IS_ERR(skb))
+ return PTR_ERR(skb);
lock_sock(sk);
@@ -1743,39 +1724,33 @@ static int hci_sock_sendmsg(struct socket *sock, struct msghdr *msg,
break;
case HCI_CHANNEL_MONITOR:
err = -EOPNOTSUPP;
- goto done;
+ goto drop;
case HCI_CHANNEL_LOGGING:
- err = hci_logging_frame(sk, buf, len, flags);
- goto done;
+ err = hci_logging_frame(sk, skb, flags);
+ goto drop;
default:
mutex_lock(&mgmt_chan_list_lock);
chan = __hci_mgmt_chan_find(hci_pi(sk)->channel);
if (chan)
- err = hci_mgmt_cmd(chan, sk, buf, len);
+ err = hci_mgmt_cmd(chan, sk, skb);
else
err = -EINVAL;
mutex_unlock(&mgmt_chan_list_lock);
- goto done;
+ goto drop;
}
hdev = hci_hdev_from_sock(sk);
if (IS_ERR(hdev)) {
err = PTR_ERR(hdev);
- goto done;
+ goto drop;
}
if (!test_bit(HCI_UP, &hdev->flags)) {
err = -ENETDOWN;
- goto done;
+ goto drop;
}
- skb = bt_skb_send_alloc(sk, len, flags & MSG_DONTWAIT, &err);
- if (!skb)
- goto done;
-
- memcpy(skb_put(skb, len), buf, len);
-
hci_skb_pkt_type(skb) = skb->data[0];
skb_pull(skb, 1);
@@ -1846,7 +1821,6 @@ static int hci_sock_sendmsg(struct socket *sock, struct msghdr *msg,
done:
release_sock(sk);
- kfree(buf);
return err;
drop:
--
2.31.1
^ permalink raw reply related [flat|nested] 3+ messages in thread* [PATCH v3 2/3] Bluetooth: Fix passing NULL to PTR_ERR
2021-09-15 23:01 [PATCH v3 1/3] Bluetooth: hci_sock: Replace use of memcpy_from_msg with bt_skb_sendmsg Luiz Augusto von Dentz
@ 2021-09-15 23:01 ` Luiz Augusto von Dentz
2021-09-15 23:01 ` [PATCH v3 3/3] Bluetooth: SCO: Fix sco_send_frame returning skb->len Luiz Augusto von Dentz
1 sibling, 0 replies; 3+ messages in thread
From: Luiz Augusto von Dentz @ 2021-09-15 23:01 UTC (permalink / raw)
To: linux-bluetooth
From: Luiz Augusto von Dentz <luiz.von.dentz@intel.com>
Passing NULL to PTR_ERR will result in 0 (success), also since the likes of
bt_skb_sendmsg does never return NULL it is safe to replace the instances of
IS_ERR_OR_NULL with IS_ERR when checking its return.
Reported-by: Dan Carpenter <dan.carpenter@oracle.com>
Signed-off-by: Luiz Augusto von Dentz <luiz.von.dentz@intel.com>
---
include/net/bluetooth/bluetooth.h | 2 +-
net/bluetooth/rfcomm/sock.c | 2 +-
net/bluetooth/sco.c | 2 +-
3 files changed, 3 insertions(+), 3 deletions(-)
diff --git a/include/net/bluetooth/bluetooth.h b/include/net/bluetooth/bluetooth.h
index aa221c1a27c6..3271870fd85e 100644
--- a/include/net/bluetooth/bluetooth.h
+++ b/include/net/bluetooth/bluetooth.h
@@ -496,7 +496,7 @@ static inline struct sk_buff *bt_skb_sendmmsg(struct sock *sk,
struct sk_buff *tmp;
tmp = bt_skb_sendmsg(sk, msg, len, mtu, headroom, tailroom);
- if (IS_ERR_OR_NULL(tmp)) {
+ if (IS_ERR(tmp)) {
kfree_skb(skb);
return tmp;
}
diff --git a/net/bluetooth/rfcomm/sock.c b/net/bluetooth/rfcomm/sock.c
index 5938af3e9936..4bf4ea6cbb5e 100644
--- a/net/bluetooth/rfcomm/sock.c
+++ b/net/bluetooth/rfcomm/sock.c
@@ -583,7 +583,7 @@ static int rfcomm_sock_sendmsg(struct socket *sock, struct msghdr *msg,
skb = bt_skb_sendmmsg(sk, msg, len, d->mtu, RFCOMM_SKB_HEAD_RESERVE,
RFCOMM_SKB_TAIL_RESERVE);
- if (IS_ERR_OR_NULL(skb))
+ if (IS_ERR(skb))
return PTR_ERR(skb);
sent = rfcomm_dlc_send(d, skb);
diff --git a/net/bluetooth/sco.c b/net/bluetooth/sco.c
index 446f871f11ed..f51399d1b9cb 100644
--- a/net/bluetooth/sco.c
+++ b/net/bluetooth/sco.c
@@ -733,7 +733,7 @@ static int sco_sock_sendmsg(struct socket *sock, struct msghdr *msg,
return -EOPNOTSUPP;
skb = bt_skb_sendmsg(sk, msg, len, len, 0, 0);
- if (IS_ERR_OR_NULL(skb))
+ if (IS_ERR(skb))
return PTR_ERR(skb);
lock_sock(sk);
--
2.31.1
^ permalink raw reply related [flat|nested] 3+ messages in thread* [PATCH v3 3/3] Bluetooth: SCO: Fix sco_send_frame returning skb->len
2021-09-15 23:01 [PATCH v3 1/3] Bluetooth: hci_sock: Replace use of memcpy_from_msg with bt_skb_sendmsg Luiz Augusto von Dentz
2021-09-15 23:01 ` [PATCH v3 2/3] Bluetooth: Fix passing NULL to PTR_ERR Luiz Augusto von Dentz
@ 2021-09-15 23:01 ` Luiz Augusto von Dentz
1 sibling, 0 replies; 3+ messages in thread
From: Luiz Augusto von Dentz @ 2021-09-15 23:01 UTC (permalink / raw)
To: linux-bluetooth
From: Luiz Augusto von Dentz <luiz.von.dentz@intel.com>
The skb in modified by hci_send_sco which pushes SCO headers thus
changing skb->len causing sco_sock_sendmsg to fail.
Fixes: 0771cbb3b97d ("Bluetooth: SCO: Replace use of memcpy_from_msg with bt_skb_sendmsg")
Signed-off-by: Luiz Augusto von Dentz <luiz.von.dentz@intel.com>
---
net/bluetooth/sco.c | 10 ++++++----
1 file changed, 6 insertions(+), 4 deletions(-)
diff --git a/net/bluetooth/sco.c b/net/bluetooth/sco.c
index f51399d1b9cb..8eabf41b2993 100644
--- a/net/bluetooth/sco.c
+++ b/net/bluetooth/sco.c
@@ -284,16 +284,17 @@ static int sco_connect(struct hci_dev *hdev, struct sock *sk)
static int sco_send_frame(struct sock *sk, struct sk_buff *skb)
{
struct sco_conn *conn = sco_pi(sk)->conn;
+ int len = skb->len;
/* Check outgoing MTU */
- if (skb->len > conn->mtu)
+ if (len > conn->mtu)
return -EINVAL;
- BT_DBG("sk %p len %d", sk, skb->len);
+ BT_DBG("sk %p len %d", sk, len);
hci_send_sco(conn->hcon, skb);
- return skb->len;
+ return len;
}
static void sco_recv_frame(struct sco_conn *conn, struct sk_buff *skb)
@@ -744,7 +745,8 @@ static int sco_sock_sendmsg(struct socket *sock, struct msghdr *msg,
err = -ENOTCONN;
release_sock(sk);
- if (err)
+
+ if (err < 0)
kfree_skb(skb);
return err;
}
--
2.31.1
^ permalink raw reply related [flat|nested] 3+ messages in thread
end of thread, other threads:[~2021-09-15 23:01 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-09-15 23:01 [PATCH v3 1/3] Bluetooth: hci_sock: Replace use of memcpy_from_msg with bt_skb_sendmsg Luiz Augusto von Dentz
2021-09-15 23:01 ` [PATCH v3 2/3] Bluetooth: Fix passing NULL to PTR_ERR Luiz Augusto von Dentz
2021-09-15 23:01 ` [PATCH v3 3/3] Bluetooth: SCO: Fix sco_send_frame returning skb->len Luiz Augusto von Dentz
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.