Re: [PATCH] Bluetooth: reorganize functions from hci_sock_sendmsg()
From: Luiz Augusto von Dentz <luiz.dentz@gmail.com>
Date: 2021-07-23 21:44:18
Hi Tetsuo, On Thu, Jul 22, 2021 at 12:42 AM Tetsuo Handa [off-list ref] wrote:
Since userfaultfd mechanism allows sleeping with kernel lock held, avoiding page fault with kernel lock held where possible will make the module more robust. This patch just brings memcpy_from_msg() calls to out of sock lock. Signed-off-by: Tetsuo Handa <penguin-kernel@I-love.SAKURA.ne.jp>
Reviewed-by: Luiz Augusto von Dentz <redacted>
quoted hunk ↗ jump to hunk
--- net/bluetooth/hci_sock.c | 50 +++++++++++++++++----------------------- 1 file changed, 21 insertions(+), 29 deletions(-)diff --git a/net/bluetooth/hci_sock.c b/net/bluetooth/hci_sock.c index ef7fc3e9d471..7fac44fb771f 100644 --- a/net/bluetooth/hci_sock.c +++ b/net/bluetooth/hci_sock.c@@ -1535,10 +1535,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, - struct msghdr *msg, size_t msglen) +static int hci_mgmt_cmd(struct hci_mgmt_chan *chan, struct sock *sk, void *buf, size_t msglen) { - void *buf; u8 *cp; struct mgmt_hdr *hdr; u16 opcode, index, len;@@ -1552,15 +1550,6 @@ static int hci_mgmt_cmd(struct hci_mgmt_chan *chan, struct sock *sk, if (msglen < sizeof(*hdr)) return -EINVAL; - buf = kmalloc(msglen, GFP_KERNEL); - if (!buf) - return -ENOMEM; - - if (memcpy_from_msg(buf, msg, msglen)) { - err = -EFAULT; - goto done; - } - hdr = buf; opcode = __le16_to_cpu(hdr->opcode); index = __le16_to_cpu(hdr->index);@@ -1657,11 +1646,10 @@ static int hci_mgmt_cmd(struct hci_mgmt_chan *chan, struct sock *sk, if (hdev) hci_dev_put(hdev); - kfree(buf); return err; } -static int hci_logging_frame(struct sock *sk, struct msghdr *msg, int len) +static int hci_logging_frame(struct sock *sk, void *buf, int len, unsigned int flags) { struct hci_mon_hdr *hdr; struct sk_buff *skb;@@ -1676,14 +1664,11 @@ static int hci_logging_frame(struct sock *sk, struct msghdr *msg, int len) if (len < sizeof(*hdr) + 3) return -EINVAL; - skb = bt_skb_send_alloc(sk, len, msg->msg_flags & MSG_DONTWAIT, &err); + skb = bt_skb_send_alloc(sk, len, flags & MSG_DONTWAIT, &err); if (!skb) return err; - if (memcpy_from_msg(skb_put(skb, len), msg, len)) { - err = -EFAULT; - goto drop; - } + memcpy(skb_put(skb, len), buf, len); hdr = (void *)skb->data;@@ -1753,19 +1738,28 @@ 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); - if (msg->msg_flags & MSG_OOB) + if (flags & MSG_OOB) return -EOPNOTSUPP; - if (msg->msg_flags & ~(MSG_DONTWAIT|MSG_NOSIGNAL|MSG_ERRQUEUE| - MSG_CMSG_COMPAT)) + if (flags & ~(MSG_DONTWAIT | MSG_NOSIGNAL | MSG_ERRQUEUE | MSG_CMSG_COMPAT)) return -EINVAL; if (len < 4 || len > HCI_MAX_FRAME_SIZE) return -EINVAL; + buf = kmalloc(len, GFP_KERNEL); + if (!buf) + return -ENOMEM; + if (memcpy_from_msg(buf, msg, len)) { + kfree(buf); + return -EFAULT; + } + lock_sock(sk); switch (hci_pi(sk)->channel) {@@ -1776,13 +1770,13 @@ static int hci_sock_sendmsg(struct socket *sock, struct msghdr *msg, err = -EOPNOTSUPP; goto done; case HCI_CHANNEL_LOGGING: - err = hci_logging_frame(sk, msg, len); + err = hci_logging_frame(sk, buf, len, flags); goto done; default: mutex_lock(&mgmt_chan_list_lock); chan = __hci_mgmt_chan_find(hci_pi(sk)->channel); if (chan) - err = hci_mgmt_cmd(chan, sk, msg, len); + err = hci_mgmt_cmd(chan, sk, buf, len); else err = -EINVAL;@@ -1801,14 +1795,11 @@ static int hci_sock_sendmsg(struct socket *sock, struct msghdr *msg, goto done; } - skb = bt_skb_send_alloc(sk, len, msg->msg_flags & MSG_DONTWAIT, &err); + skb = bt_skb_send_alloc(sk, len, flags & MSG_DONTWAIT, &err); if (!skb) goto done; - if (memcpy_from_msg(skb_put(skb, len), msg, len)) { - err = -EFAULT; - goto drop; - } + memcpy(skb_put(skb, len), buf, len); hci_skb_pkt_type(skb) = skb->data[0]; skb_pull(skb, 1);@@ -1880,6 +1871,7 @@ static int hci_sock_sendmsg(struct socket *sock, struct msghdr *msg, done: release_sock(sk); + kfree(buf); return err; drop: --2.18.4
-- Luiz Augusto von Dentz