| Message ID | 20210722074208.8040-1-penguin-kernel@I-love.SAKURA.ne.jp (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | Bluetooth: reorganize functions from hci_sock_sendmsg() | expand |
This is automated email and please do not reply to this email! Dear submitter, Thank you for submitting the patches to the linux bluetooth mailing list. This is a CI test results with your patch series: PW Link:https://patchwork.kernel.org/project/bluetooth/list/?series=519543 ---Test result--- Test Summary: CheckPatch PASS 0.48 seconds GitLint PASS 0.10 seconds BuildKernel PASS 531.77 seconds TestRunner: Setup PASS 350.46 seconds TestRunner: l2cap-tester PASS 2.52 seconds TestRunner: bnep-tester PASS 1.88 seconds TestRunner: mgmt-tester PASS 31.41 seconds TestRunner: rfcomm-tester PASS 2.11 seconds TestRunner: sco-tester PASS 2.03 seconds TestRunner: smp-tester FAIL 2.10 seconds TestRunner: userchan-tester PASS 1.94 seconds Details ############################## Test: CheckPatch - PASS - 0.48 seconds Run checkpatch.pl script with rule in .checkpatch.conf ############################## Test: GitLint - PASS - 0.10 seconds Run gitlint with rule in .gitlint ############################## Test: BuildKernel - PASS - 531.77 seconds Build Kernel with minimal configuration supports Bluetooth ############################## Test: TestRunner: Setup - PASS - 350.46 seconds Setup environment for running Test Runner ############################## Test: TestRunner: l2cap-tester - PASS - 2.52 seconds Run test-runner with l2cap-tester Total: 40, Passed: 40 (100.0%), Failed: 0, Not Run: 0 ############################## Test: TestRunner: bnep-tester - PASS - 1.88 seconds Run test-runner with bnep-tester Total: 1, Passed: 1 (100.0%), Failed: 0, Not Run: 0 ############################## Test: TestRunner: mgmt-tester - PASS - 31.41 seconds Run test-runner with mgmt-tester Total: 448, Passed: 445 (99.3%), Failed: 0, Not Run: 3 ############################## Test: TestRunner: rfcomm-tester - PASS - 2.11 seconds Run test-runner with rfcomm-tester Total: 9, Passed: 9 (100.0%), Failed: 0, Not Run: 0 ############################## Test: TestRunner: sco-tester - PASS - 2.03 seconds Run test-runner with sco-tester Total: 8, Passed: 8 (100.0%), Failed: 0, Not Run: 0 ############################## Test: TestRunner: smp-tester - FAIL - 2.10 seconds Run test-runner with smp-tester Total: 8, Passed: 7 (87.5%), Failed: 1, Not Run: 0 Failed Test Cases SMP Client - SC Request 2 Failed 0.023 seconds ############################## Test: TestRunner: userchan-tester - PASS - 1.94 seconds Run test-runner with userchan-tester Total: 3, Passed: 3 (100.0%), Failed: 0, Not Run: 0 --- Regards, Linux Bluetooth
Hi Tetsuo, On Thu, Jul 22, 2021 at 12:42 AM Tetsuo Handa <penguin-kernel@i-love.sakura.ne.jp> 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 <luiz.von.dentz@intel.com> > --- > 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 >
On 2021/07/24 6:44, Luiz Augusto von Dentz wrote: > Hi Tetsuo, > > On Thu, Jul 22, 2021 at 12:42 AM Tetsuo Handa > <penguin-kernel@i-love.sakura.ne.jp> 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 <luiz.von.dentz@intel.com> > Can this patch go to v5.15 ?
Hi Tetsuo, On Sun, Aug 22, 2021 at 8:42 AM Tetsuo Handa <penguin-kernel@i-love.sakura.ne.jp> wrote: > > On 2021/07/24 6:44, Luiz Augusto von Dentz wrote: > > Hi Tetsuo, > > > > On Thu, Jul 22, 2021 at 12:42 AM Tetsuo Handa > > <penguin-kernel@i-love.sakura.ne.jp> 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 <luiz.von.dentz@intel.com> > > > > Can this patch go to v5.15 ? Applied, thanks. I will be sending another pull-request by the end of this week which shall include it.
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:
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> --- net/bluetooth/hci_sock.c | 50 +++++++++++++++++----------------------- 1 file changed, 21 insertions(+), 29 deletions(-)