Hi Tetsuo, On Thu, Jul 22, 2021 at 12:42 AM Tetsuo Handa <penguin-kernel@xxxxxxxxxxxxxxxxxxx> 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@xxxxxxxxxxxxxxxxxxx> Reviewed-by: Luiz Augusto von Dentz <luiz.von.dentz@xxxxxxxxx> > --- > 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