From 3fc0b380c6db42337e651f4f619534c4d4f5915f Mon Sep 17 00:00:00 2001 From: emax Date: Fri, 30 Aug 2019 16:35:31 +0000 Subject: [PATCH] avoid holding PCB mutex during copyin/copyout() Reported by: imp, mms dot vanbreukelingen at gmail dot com Reviewed by: imp --- .../bluetooth/socket/ng_btsocket_hci_raw.c | 101 ++++++++----- .../bluetooth/socket/ng_btsocket_l2cap_raw.c | 142 +++++++++++------- 2 files changed, 154 insertions(+), 89 deletions(-) diff --git a/sys/netgraph/bluetooth/socket/ng_btsocket_hci_raw.c b/sys/netgraph/bluetooth/socket/ng_btsocket_hci_raw.c index 9523d88af633..defae9280d52 100644 --- a/sys/netgraph/bluetooth/socket/ng_btsocket_hci_raw.c +++ b/sys/netgraph/bluetooth/socket/ng_btsocket_hci_raw.c @@ -1156,15 +1156,15 @@ ng_btsocket_hci_raw_control(struct socket *so, u_long cmd, caddr_t data, 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 cmd, caddr_t data, 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 cmd, caddr_t data, 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 cmd, caddr_t data, } 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 cmd, caddr_t data, 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 cmd, caddr_t data, 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 cmd, caddr_t data, 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 cmd, caddr_t data, } 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 cmd, caddr_t data, 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 cmd, caddr_t data, 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 cmd, caddr_t data, 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 cmd, caddr_t data, } 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; diff --git a/sys/netgraph/bluetooth/socket/ng_btsocket_l2cap_raw.c b/sys/netgraph/bluetooth/socket/ng_btsocket_l2cap_raw.c index a31bae225fce..577856ab487f 100644 --- a/sys/netgraph/bluetooth/socket/ng_btsocket_l2cap_raw.c +++ b/sys/netgraph/bluetooth/socket/ng_btsocket_l2cap_raw.c @@ -852,15 +852,15 @@ ng_btsocket_l2cap_raw_control(struct socket *so, u_long cmd, caddr_t data, 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_long cmd, caddr_t data, 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_long cmd, caddr_t data, } 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_long cmd, caddr_t data, 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_long cmd, caddr_t data, 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_long cmd, caddr_t data, } 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_long cmd, caddr_t data, 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_long cmd, caddr_t data, 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_long cmd, caddr_t data, 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_long cmd, caddr_t data, } 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_long cmd, caddr_t data, 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_long cmd, caddr_t data, 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_long cmd, caddr_t data, } 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 =