Author: emax
Date: Fri Aug 30 16:35:31 2019
New Revision: 351626
URL: https://svnweb.freebsd.org/changeset/base/351626

Log:
  avoid holding PCB mutex during copyin/copyout()
  
  Reported by:  imp, mms dot vanbreukelingen at gmail dot com
  Reviewed by:  imp

Modified:
  head/sys/netgraph/bluetooth/socket/ng_btsocket_hci_raw.c
  head/sys/netgraph/bluetooth/socket/ng_btsocket_l2cap_raw.c

Modified: head/sys/netgraph/bluetooth/socket/ng_btsocket_hci_raw.c
==============================================================================
--- head/sys/netgraph/bluetooth/socket/ng_btsocket_hci_raw.c    Fri Aug 30 
16:30:09 2019        (r351625)
+++ head/sys/netgraph/bluetooth/socket/ng_btsocket_hci_raw.c    Fri Aug 30 
16:35:31 2019        (r351626)
@@ -1156,15 +1156,15 @@ ng_btsocket_hci_raw_control(struct socket *so, u_long 
                if (p->num_entries <= 0 || 
                    p->num_entries > NG_HCI_MAX_NEIGHBOR_NUM ||
                    p->entries == NULL) {
-                       error = EINVAL;
-                       break;
+                       mtx_unlock(&pcb->pcb_mtx);
+                       return (EINVAL);
                }
 
                NG_MKMESSAGE(msg, NGM_HCI_COOKIE,
                        NGM_HCI_NODE_GET_NEIGHBOR_CACHE, 0, M_NOWAIT);
                if (msg == NULL) {
-                       error = ENOMEM;
-                       break;
+                       mtx_unlock(&pcb->pcb_mtx);
+                       return (ENOMEM);
                }
                ng_btsocket_hci_raw_get_token(&msg->header.token);
                pcb->token = msg->header.token;
@@ -1173,7 +1173,8 @@ ng_btsocket_hci_raw_control(struct socket *so, u_long 
                NG_SEND_MSG_PATH(error, ng_btsocket_hci_raw_node, msg, path, 0);
                if (error != 0) {
                        pcb->token = 0;
-                       break;
+                       mtx_unlock(&pcb->pcb_mtx);
+                       return (error);
                }
 
                error = msleep(&pcb->msg, &pcb->pcb_mtx,
@@ -1181,16 +1182,21 @@ ng_btsocket_hci_raw_control(struct socket *so, u_long 
                                ng_btsocket_hci_raw_ioctl_timeout * hz);
                pcb->token = 0;
 
-               if (error != 0)
-                       break;
+               if (error != 0) {
+                       mtx_unlock(&pcb->pcb_mtx);
+                       return (error);
+               }
 
-               if (pcb->msg != NULL &&
-                   pcb->msg->header.cmd == NGM_HCI_NODE_GET_NEIGHBOR_CACHE) {
+               msg = pcb->msg;
+               pcb->msg = NULL;
+
+               mtx_unlock(&pcb->pcb_mtx);
+               
+               if (msg != NULL &&
+                   msg->header.cmd == NGM_HCI_NODE_GET_NEIGHBOR_CACHE) {
                        /* Return data back to user space */
-                       p1 = (ng_hci_node_get_neighbor_cache_ep *)
-                               (pcb->msg->data);
-                       p2 = (ng_hci_node_neighbor_cache_entry_ep *)
-                               (p1 + 1);
+                       p1 = (ng_hci_node_get_neighbor_cache_ep *)(msg->data);
+                       p2 = (ng_hci_node_neighbor_cache_entry_ep *)(p1 + 1);
 
                        p->num_entries = min(p->num_entries, p1->num_entries);
                        if (p->num_entries > 0)
@@ -1200,8 +1206,9 @@ ng_btsocket_hci_raw_control(struct socket *so, u_long 
                } else
                        error = EINVAL;
 
-               NG_FREE_MSG(pcb->msg); /* checks for != NULL */
-               }break;
+               NG_FREE_MSG(msg); /* checks for != NULL */
+               return (error);
+               } /* NOTREACHED */
 
        case SIOC_HCI_RAW_NODE_GET_CON_LIST: {
                struct ng_btsocket_hci_raw_con_list     *p = 
@@ -1212,15 +1219,15 @@ ng_btsocket_hci_raw_control(struct socket *so, u_long 
                if (p->num_connections == 0 ||
                    p->num_connections > NG_HCI_MAX_CON_NUM ||
                    p->connections == NULL) {
-                       error = EINVAL;
-                       break;
+                       mtx_unlock(&pcb->pcb_mtx);
+                       return (EINVAL);
                }
 
                NG_MKMESSAGE(msg, NGM_HCI_COOKIE, NGM_HCI_NODE_GET_CON_LIST,
                        0, M_NOWAIT);
                if (msg == NULL) {
-                       error = ENOMEM;
-                       break;
+                       mtx_unlock(&pcb->pcb_mtx);
+                       return (ENOMEM);
                }
                ng_btsocket_hci_raw_get_token(&msg->header.token);
                pcb->token = msg->header.token;
@@ -1229,7 +1236,8 @@ ng_btsocket_hci_raw_control(struct socket *so, u_long 
                NG_SEND_MSG_PATH(error, ng_btsocket_hci_raw_node, msg, path, 0);
                if (error != 0) {
                        pcb->token = 0;
-                       break;
+                       mtx_unlock(&pcb->pcb_mtx);
+                       return (error);
                }
 
                error = msleep(&pcb->msg, &pcb->pcb_mtx,
@@ -1237,13 +1245,20 @@ ng_btsocket_hci_raw_control(struct socket *so, u_long 
                                ng_btsocket_hci_raw_ioctl_timeout * hz);
                pcb->token = 0;
 
-               if (error != 0)
-                       break;
+               if (error != 0) {
+                       mtx_unlock(&pcb->pcb_mtx);
+                       return (error);
+               }
 
-               if (pcb->msg != NULL &&
-                   pcb->msg->header.cmd == NGM_HCI_NODE_GET_CON_LIST) {
+               msg = pcb->msg;
+               pcb->msg = NULL;
+
+               mtx_unlock(&pcb->pcb_mtx);
+
+               if (msg != NULL &&
+                   msg->header.cmd == NGM_HCI_NODE_GET_CON_LIST) {
                        /* Return data back to user space */
-                       p1 = (ng_hci_node_con_list_ep *)(pcb->msg->data);
+                       p1 = (ng_hci_node_con_list_ep *)(msg->data);
                        p2 = (ng_hci_node_con_ep *)(p1 + 1);
 
                        p->num_connections = min(p->num_connections,
@@ -1255,8 +1270,9 @@ ng_btsocket_hci_raw_control(struct socket *so, u_long 
                } else
                        error = EINVAL;
 
-               NG_FREE_MSG(pcb->msg); /* checks for != NULL */
-               } break;
+               NG_FREE_MSG(msg); /* checks for != NULL */
+               return (error);
+               } /* NOTREACHED */
 
        case SIOC_HCI_RAW_NODE_GET_LINK_POLICY_MASK: {
                struct ng_btsocket_hci_raw_node_link_policy_mask        *p = 
@@ -1332,15 +1348,15 @@ ng_btsocket_hci_raw_control(struct socket *so, u_long 
                struct nodeinfo                                 *ni = nl->names;
 
                if (nl->num_names == 0) {
-                       error = EINVAL;
-                       break;
+                       mtx_unlock(&pcb->pcb_mtx);
+                       return (EINVAL);
                }
 
                NG_MKMESSAGE(msg, NGM_GENERIC_COOKIE, NGM_LISTNAMES,
                        0, M_NOWAIT);
                if (msg == NULL) {
-                       error = ENOMEM;
-                       break;
+                       mtx_unlock(&pcb->pcb_mtx);
+                       return (ENOMEM);
                }
                ng_btsocket_hci_raw_get_token(&msg->header.token);
                pcb->token = msg->header.token;
@@ -1349,7 +1365,8 @@ ng_btsocket_hci_raw_control(struct socket *so, u_long 
                NG_SEND_MSG_PATH(error, ng_btsocket_hci_raw_node, msg, ".:", 0);
                if (error != 0) {
                        pcb->token = 0;
-                       break;
+                       mtx_unlock(&pcb->pcb_mtx);
+                       return (error);
                }
 
                error = msleep(&pcb->msg, &pcb->pcb_mtx,
@@ -1357,12 +1374,19 @@ ng_btsocket_hci_raw_control(struct socket *so, u_long 
                                ng_btsocket_hci_raw_ioctl_timeout * hz);
                pcb->token = 0;
 
-               if (error != 0)
-                       break;
+               if (error != 0) {
+                       mtx_unlock(&pcb->pcb_mtx);
+                       return (error);
+               }
 
-               if (pcb->msg != NULL && pcb->msg->header.cmd == NGM_LISTNAMES) {
+               msg = pcb->msg;
+               pcb->msg = NULL;
+
+               mtx_unlock(&pcb->pcb_mtx);
+
+               if (msg != NULL && msg->header.cmd == NGM_LISTNAMES) {
                        /* Return data back to user space */
-                       struct namelist *nl1 = (struct namelist *) 
pcb->msg->data;
+                       struct namelist *nl1 = (struct namelist *) msg->data;
                        struct nodeinfo *ni1 = &nl1->nodeinfo[0];
 
                        while (nl->num_names > 0 && nl1->numnames > 0) {
@@ -1385,8 +1409,9 @@ ng_btsocket_hci_raw_control(struct socket *so, u_long 
                } else
                        error = EINVAL;
 
-               NG_FREE_MSG(pcb->msg); /* checks for != NULL */
-               } break;
+               NG_FREE_MSG(msg); /* checks for != NULL */
+               return (error);
+               } /* NOTREACHED */
 
        default:
                error = EINVAL;

Modified: head/sys/netgraph/bluetooth/socket/ng_btsocket_l2cap_raw.c
==============================================================================
--- head/sys/netgraph/bluetooth/socket/ng_btsocket_l2cap_raw.c  Fri Aug 30 
16:30:09 2019        (r351625)
+++ head/sys/netgraph/bluetooth/socket/ng_btsocket_l2cap_raw.c  Fri Aug 30 
16:35:31 2019        (r351626)
@@ -852,15 +852,15 @@ ng_btsocket_l2cap_raw_control(struct socket *so, u_lon
                if (p->num_connections == 0 ||
                    p->num_connections > NG_L2CAP_MAX_CON_NUM ||
                    p->connections == NULL) {
-                       error = EINVAL;
-                       break;
+                       mtx_unlock(&pcb->pcb_mtx);
+                       return (EINVAL);
                }
 
                NG_MKMESSAGE(msg, NGM_L2CAP_COOKIE, NGM_L2CAP_NODE_GET_CON_LIST,
                        0, M_NOWAIT);
                if (msg == NULL) {
-                       error = ENOMEM;
-                       break;
+                       mtx_unlock(&pcb->pcb_mtx);
+                       return (ENOMEM);
                }
                ng_btsocket_l2cap_raw_get_token(&msg->header.token);
                pcb->token = msg->header.token;
@@ -870,20 +870,28 @@ ng_btsocket_l2cap_raw_control(struct socket *so, u_lon
                        pcb->rt->hook, 0);
                if (error != 0) {
                        pcb->token = 0;
-                       break;
+                       mtx_unlock(&pcb->pcb_mtx);
+                       return (error);
                }
 
                error = msleep(&pcb->msg, &pcb->pcb_mtx, PZERO|PCATCH, "l2ctl",
                                ng_btsocket_l2cap_raw_ioctl_timeout * hz);
                pcb->token = 0;
 
-               if (error != 0)
-                       break;
+               if (error != 0) {
+                       mtx_unlock(&pcb->pcb_mtx);
+                       return (error);
+               }
 
-               if (pcb->msg != NULL &&
-                   pcb->msg->header.cmd == NGM_L2CAP_NODE_GET_CON_LIST) {
+               msg = pcb->msg;
+               pcb->msg = NULL;
+
+               mtx_unlock(&pcb->pcb_mtx);
+
+               if (msg != NULL &&
+                   msg->header.cmd == NGM_L2CAP_NODE_GET_CON_LIST) {
                        /* Return data back to user space */
-                       p1 = (ng_l2cap_node_con_list_ep *)(pcb->msg->data);
+                       p1 = (ng_l2cap_node_con_list_ep *)(msg->data);
                        p2 = (ng_l2cap_node_con_ep *)(p1 + 1);
 
                        p->num_connections = min(p->num_connections,
@@ -895,8 +903,9 @@ ng_btsocket_l2cap_raw_control(struct socket *so, u_lon
                } else
                        error = EINVAL;
 
-               NG_FREE_MSG(pcb->msg); /* checks for != NULL */
-               } break;
+               NG_FREE_MSG(msg); /* checks for != NULL */
+               return (error);
+               } /* NOTREACHED */
 
        case SIOC_L2CAP_NODE_GET_CHAN_LIST: {
                struct ng_btsocket_l2cap_raw_chan_list  *p =
@@ -907,15 +916,15 @@ ng_btsocket_l2cap_raw_control(struct socket *so, u_lon
                if (p->num_channels == 0 ||
                    p->num_channels > NG_L2CAP_MAX_CHAN_NUM ||
                    p->channels == NULL) {
-                       error = EINVAL;
-                       break;
+                       mtx_unlock(&pcb->pcb_mtx);
+                       return (EINVAL);
                }
  
                NG_MKMESSAGE(msg, NGM_L2CAP_COOKIE,
                        NGM_L2CAP_NODE_GET_CHAN_LIST, 0, M_NOWAIT);
                if (msg == NULL) {
-                       error = ENOMEM;
-                       break;
+                       mtx_unlock(&pcb->pcb_mtx);
+                       return (ENOMEM);
                }
                ng_btsocket_l2cap_raw_get_token(&msg->header.token);
                pcb->token = msg->header.token;
@@ -925,20 +934,28 @@ ng_btsocket_l2cap_raw_control(struct socket *so, u_lon
                        pcb->rt->hook, 0);
                if (error != 0) {
                        pcb->token = 0;
-                       break;
+                       mtx_unlock(&pcb->pcb_mtx);
+                       return (error);
                }
 
                error = msleep(&pcb->msg, &pcb->pcb_mtx, PZERO|PCATCH, "l2ctl",
                                ng_btsocket_l2cap_raw_ioctl_timeout * hz);
                pcb->token = 0;
 
-               if (error != 0)
-                       break;
+               if (error != 0) {
+                       mtx_unlock(&pcb->pcb_mtx);
+                       return (error);
+               }
 
-               if (pcb->msg != NULL &&
-                   pcb->msg->header.cmd == NGM_L2CAP_NODE_GET_CHAN_LIST) {
+               msg = pcb->msg;
+               pcb->msg = NULL;
+
+               mtx_unlock(&pcb->pcb_mtx);
+
+               if (msg != NULL &&
+                   msg->header.cmd == NGM_L2CAP_NODE_GET_CHAN_LIST) {
                        /* Return data back to user space */
-                       p1 = (ng_l2cap_node_chan_list_ep *)(pcb->msg->data);
+                       p1 = (ng_l2cap_node_chan_list_ep *)(msg->data);
                        p2 = (ng_l2cap_node_chan_ep *)(p1 + 1);
 
                        p->num_channels = min(p->num_channels, 
@@ -950,8 +967,9 @@ ng_btsocket_l2cap_raw_control(struct socket *so, u_lon
                } else
                        error = EINVAL;
 
-               NG_FREE_MSG(pcb->msg); /* checks for != NULL */
-               } break;
+               NG_FREE_MSG(msg); /* checks for != NULL */
+               return (error);
+               } /* NOTREACHED */
 
        case SIOC_L2CAP_L2CA_PING: {
                struct ng_btsocket_l2cap_raw_ping       *p = 
@@ -961,16 +979,16 @@ ng_btsocket_l2cap_raw_control(struct socket *so, u_lon
 
                if ((p->echo_size != 0 && p->echo_data == NULL) ||
                     p->echo_size > NG_L2CAP_MAX_ECHO_SIZE) {
-                       error = EINVAL;
-                       break;
+                       mtx_unlock(&pcb->pcb_mtx);
+                       return (EINVAL);
                }
 
                NG_MKMESSAGE(msg, NGM_L2CAP_COOKIE,
                        NGM_L2CAP_L2CA_PING, sizeof(*ip) + p->echo_size,
                        M_NOWAIT);
                if (msg == NULL) {
-                       error = ENOMEM;
-                       break;
+                       mtx_unlock(&pcb->pcb_mtx);
+                       return (ENOMEM);
                }
                ng_btsocket_l2cap_raw_get_token(&msg->header.token);
                pcb->token = msg->header.token;
@@ -981,11 +999,15 @@ ng_btsocket_l2cap_raw_control(struct socket *so, u_lon
                ip->echo_size = p->echo_size;
 
                if (ip->echo_size > 0) {
+                       mtx_unlock(&pcb->pcb_mtx);
                        error = copyin(p->echo_data, ip + 1, p->echo_size);
+                       mtx_lock(&pcb->pcb_mtx);
+
                        if (error != 0) {
                                NG_FREE_MSG(msg);
                                pcb->token = 0;
-                               break;
+                               mtx_unlock(&pcb->pcb_mtx);
+                               return (error);
                        }
                }
 
@@ -993,20 +1015,28 @@ ng_btsocket_l2cap_raw_control(struct socket *so, u_lon
                        pcb->rt->hook, 0);
                if (error != 0) {
                        pcb->token = 0;
-                       break;
+                       mtx_unlock(&pcb->pcb_mtx);
+                       return (error);
                }
 
                error = msleep(&pcb->msg, &pcb->pcb_mtx, PZERO|PCATCH, "l2ctl",
                                bluetooth_l2cap_rtx_timeout());
                pcb->token = 0;
 
-               if (error != 0)
-                       break;
+               if (error != 0) {
+                       mtx_unlock(&pcb->pcb_mtx);
+                       return (error);
+               }
 
-               if (pcb->msg != NULL &&
-                   pcb->msg->header.cmd == NGM_L2CAP_L2CA_PING) {
+               msg = pcb->msg;
+               pcb->msg = NULL;
+
+               mtx_unlock(&pcb->pcb_mtx);
+
+               if (msg != NULL &&
+                   msg->header.cmd == NGM_L2CAP_L2CA_PING) {
                        /* Return data back to the user space */
-                       op = (ng_l2cap_l2ca_ping_op *)(pcb->msg->data);
+                       op = (ng_l2cap_l2ca_ping_op *)(msg->data);
                        p->result = op->result;
                        p->echo_size = min(p->echo_size, op->echo_size);
 
@@ -1016,8 +1046,9 @@ ng_btsocket_l2cap_raw_control(struct socket *so, u_lon
                } else
                        error = EINVAL;
 
-               NG_FREE_MSG(pcb->msg); /* checks for != NULL */
-               } break;
+               NG_FREE_MSG(msg); /* checks for != NULL */
+               return (error);
+               } /* NOTREACHED */
 
        case SIOC_L2CAP_L2CA_GET_INFO: {
                struct ng_btsocket_l2cap_raw_get_info   *p = 
@@ -1026,21 +1057,21 @@ ng_btsocket_l2cap_raw_control(struct socket *so, u_lon
                ng_l2cap_l2ca_get_info_op               *op = NULL;
 
                if (!(pcb->flags & NG_BTSOCKET_L2CAP_RAW_PRIVILEGED)) {
-                       error = EPERM;
-                       break;
+                       mtx_unlock(&pcb->pcb_mtx);
+                       return (EPERM);
                }
 
                if (p->info_size != 0 && p->info_data == NULL) {
-                       error = EINVAL;
-                       break;
+                       mtx_unlock(&pcb->pcb_mtx);
+                       return (EINVAL);
                }
 
                NG_MKMESSAGE(msg, NGM_L2CAP_COOKIE,
                        NGM_L2CAP_L2CA_GET_INFO, sizeof(*ip) + p->info_size,
                        M_NOWAIT);
                if (msg == NULL) {
-                       error = ENOMEM;
-                       break;
+                       mtx_unlock(&pcb->pcb_mtx);
+                       return (ENOMEM);
                }
                ng_btsocket_l2cap_raw_get_token(&msg->header.token);
                pcb->token = msg->header.token;
@@ -1054,20 +1085,28 @@ ng_btsocket_l2cap_raw_control(struct socket *so, u_lon
                        pcb->rt->hook, 0);
                if (error != 0) {
                        pcb->token = 0;
-                       break;
+                       mtx_unlock(&pcb->pcb_mtx);
+                       return (error);
                }
 
                error = msleep(&pcb->msg, &pcb->pcb_mtx, PZERO|PCATCH, "l2ctl",
                                bluetooth_l2cap_rtx_timeout());
                pcb->token = 0;
 
-               if (error != 0)
-                       break;
+               if (error != 0) {
+                       mtx_unlock(&pcb->pcb_mtx);
+                       return (error);
+               }
 
-               if (pcb->msg != NULL &&
-                   pcb->msg->header.cmd == NGM_L2CAP_L2CA_GET_INFO) {
+               msg = pcb->msg;
+               pcb->msg = NULL;
+
+               mtx_unlock(&pcb->pcb_mtx);
+
+               if (msg != NULL &&
+                   msg->header.cmd == NGM_L2CAP_L2CA_GET_INFO) {
                        /* Return data back to the user space */
-                       op = (ng_l2cap_l2ca_get_info_op *)(pcb->msg->data);
+                       op = (ng_l2cap_l2ca_get_info_op *)(msg->data);
                        p->result = op->result;
                        p->info_size = min(p->info_size, op->info_size);
 
@@ -1077,8 +1116,9 @@ ng_btsocket_l2cap_raw_control(struct socket *so, u_lon
                } else
                        error = EINVAL;
 
-               NG_FREE_MSG(pcb->msg); /* checks for != NULL */
-               } break;
+               NG_FREE_MSG(msg); /* checks for != NULL */
+               return (error);
+               } /* NOTREACHED */
 
        case SIOC_L2CAP_NODE_GET_AUTO_DISCON_TIMO: {
                struct ng_btsocket_l2cap_raw_auto_discon_timo   *p =
_______________________________________________
svn-src-head@freebsd.org mailing list
https://lists.freebsd.org/mailman/listinfo/svn-src-head
To unsubscribe, send any mail to "svn-src-head-unsubscr...@freebsd.org"

Reply via email to