diff options
| author | Alexander Bluhm <bluhm@cvs.openbsd.org> | 2023-11-13 17:18:28 +0000 |
|---|---|---|
| committer | Alexander Bluhm <bluhm@cvs.openbsd.org> | 2023-11-13 17:18:28 +0000 |
| commit | ec40add2bf0ca7a0db877aae359bccb5651b71e4 (patch) | |
| tree | d6e93294289d83b494a08d9b008d04ed8714a03c /sys/net | |
| parent | 4958b59d9028b6ea8067d173ae06382e7453ec3e (diff) | |
Fix rt_setgate() error handling.
In revision 1.424 the logic in rt_setgate() has changed. The old
code entered a value into rt_gateway also if rt_setgwroute() returned
an error. Now if rt_setgwroute() fails, rt_gateway is NULL and
ROUNDUP(rt->rt_gateway->sa_len) crashes.
Put back the old logic in rt_setgate(). Setting rt_gateway and
rt_gwroute are actually independent.
If malloc(9) in rt_setgate() fails, rt_gateway can still be NULL.
The subsequent crash in free(rt->rt_gateway, M_RTABLE,
ROUNDUP(rt->rt_gateway->sa_len)) was just never observed. Add a
NULL check around these free(9).
Reported-by: syzbot+2e79dd9db712d3c5ade9@syzkaller.appspotmail.com
OK mvs@
Diffstat (limited to 'sys/net')
| -rw-r--r-- | sys/net/route.c | 34 |
1 files changed, 18 insertions, 16 deletions
diff --git a/sys/net/route.c b/sys/net/route.c index f6930ba73ee..0b97254765f 100644 --- a/sys/net/route.c +++ b/sys/net/route.c @@ -1,4 +1,4 @@ -/* $OpenBSD: route.c,v 1.425 2023/11/12 17:51:40 bluhm Exp $ */ +/* $OpenBSD: route.c,v 1.426 2023/11/13 17:18:27 bluhm Exp $ */ /* $NetBSD: route.c,v 1.14 1996/02/13 22:00:46 christos Exp $ */ /* @@ -517,7 +517,10 @@ rtfree(struct rtentry *rt) #ifdef MPLS rt_mpls_clear(rt); #endif - free(rt->rt_gateway, M_RTABLE, ROUNDUP(rt->rt_gateway->sa_len)); + if (rt->rt_gateway != NULL) { + free(rt->rt_gateway, M_RTABLE, + ROUNDUP(rt->rt_gateway->sa_len)); + } free(rt_key(rt), M_RTABLE, rt_key(rt)->sa_len); pool_put(&rtentry_pool, rt); @@ -937,8 +940,10 @@ rtrequest(int req, struct rt_addrinfo *info, u_int8_t prio, ifafree(ifa); rtfree(rt->rt_parent); rt_putgwroute(rt, NULL); - free(rt->rt_gateway, M_RTABLE, - ROUNDUP(rt->rt_gateway->sa_len)); + if (rt->rt_gateway != NULL) { + free(rt->rt_gateway, M_RTABLE, + ROUNDUP(rt->rt_gateway->sa_len)); + } free(ndst, M_RTABLE, ndst->sa_len); pool_put(&rtentry_pool, rt); return (error); @@ -970,8 +975,10 @@ rtrequest(int req, struct rt_addrinfo *info, u_int8_t prio, ifafree(ifa); rtfree(rt->rt_parent); rt_putgwroute(rt, NULL); - free(rt->rt_gateway, M_RTABLE, - ROUNDUP(rt->rt_gateway->sa_len)); + if (rt->rt_gateway != NULL) { + free(rt->rt_gateway, M_RTABLE, + ROUNDUP(rt->rt_gateway->sa_len)); + } free(ndst, M_RTABLE, ndst->sa_len); pool_put(&rtentry_pool, rt); return (EEXIST); @@ -996,6 +1003,7 @@ rt_setgate(struct rtentry *rt, const struct sockaddr *gate, u_int rtableid) { int glen = ROUNDUP(gate->sa_len); struct sockaddr *sa, *osa; + int error = 0; KASSERT(gate != NULL); if (rt->rt_gateway == gate) { @@ -1009,23 +1017,17 @@ rt_setgate(struct rtentry *rt, const struct sockaddr *gate, u_int rtableid) memcpy(sa, gate, gate->sa_len); KERNEL_LOCK(); /* see [X] in route.h */ - if (ISSET(rt->rt_flags, RTF_GATEWAY)) { - int error = rt_setgwroute(rt, gate, rtableid); - if (error != 0) { - KERNEL_UNLOCK(); - free(sa, M_RTABLE, glen); - return (error); - } - } - osa = rt->rt_gateway; rt->rt_gateway = sa; + + if (ISSET(rt->rt_flags, RTF_GATEWAY)) + error = rt_setgwroute(rt, gate, rtableid); KERNEL_UNLOCK(); if (osa != NULL) free(osa, M_RTABLE, ROUNDUP(osa->sa_len)); - return (0); + return (error); } /* |