Download raw body.
sysctl(2): unlock ipip_sysctl()
On Tue, Aug 20, 2024 at 05:54:03PM +0300, Vitaliy Makkoveev wrote:
> - IPIPCTL_ALLOW - atomically accessed integer;
> - IPIPCTL_STATS - per-CPU counters;
>
> ok?
ipip_input() calls ipip_input_if(). ipip_allow is read in both
functions without lock which can cause inconsistent behaviour.
ipip_allow = 2
ipip_input() passes packet
ipip_allow = 0
ipip_input_if() does local address spoofing check
This code should only be reached if ipip_allow == 1
You have to load ipip_allow in ipip_input() and pass value or flags
to ipip_input_if().
bluhm
> Index: sys/netinet/in_proto.c
> ===================================================================
> RCS file: /cvs/src/sys/netinet/in_proto.c,v
> diff -u -p -r1.110 in_proto.c
> --- sys/netinet/in_proto.c 20 Aug 2024 07:47:25 -0000 1.110
> +++ sys/netinet/in_proto.c 20 Aug 2024 14:51:59 -0000
> @@ -230,7 +230,7 @@ const struct protosw inetsw[] = {
> .pr_type = SOCK_RAW,
> .pr_domain = &inetdomain,
> .pr_protocol = IPPROTO_IPV4,
> - .pr_flags = PR_ATOMIC|PR_ADDR|PR_MPSOCKET,
> + .pr_flags = PR_ATOMIC|PR_ADDR|PR_MPSOCKET|PR_MPSYSCTL,
> #if NGIF > 0
> .pr_input = in_gif_input,
> #else
> Index: sys/netinet/ip_ipip.c
> ===================================================================
> RCS file: /cvs/src/sys/netinet/ip_ipip.c,v
> diff -u -p -r1.102 ip_ipip.c
> --- sys/netinet/ip_ipip.c 17 May 2024 20:44:36 -0000 1.102
> +++ sys/netinet/ip_ipip.c 20 Aug 2024 14:51:59 -0000
> @@ -72,6 +72,11 @@
> #include <net/pfvar.h>
> #endif
>
> +/*
> + * Locks used to protect data:
> + * a atomic
> + */
> +
> #ifdef ENCDEBUG
> #define DPRINTF(fmt, args...) \
> do { \
> @@ -87,7 +92,7 @@
> * We can control the acceptance of IP4 packets by altering the sysctl
> * net.inet.ipip.allow value. Zero means drop them, all else is acceptance.
> */
> -int ipip_allow = 0;
> +int ipip_allow = 0; /* [a] */
>
> struct cpumem *ipipcounters;
>
> @@ -106,7 +111,8 @@ ipip_input(struct mbuf **mp, int *offp,
> struct ifnet *ifp;
>
> /* If we do not accept IP-in-IP explicitly, drop. */
> - if (!ipip_allow && ((*mp)->m_flags & (M_AUTH|M_CONF)) == 0) {
> + if (atomic_load_int(&ipip_allow) == 0 &&
> + ((*mp)->m_flags & (M_AUTH|M_CONF)) == 0) {
> DPRINTF("dropped due to policy");
> ipipstat_inc(ipips_pdrops);
> m_freemp(mp);
> @@ -271,7 +277,8 @@ ipip_input_if(struct mbuf **mp, int *off
> }
>
> /* Check for local address spoofing. */
> - if (!(ifp->if_flags & IFF_LOOPBACK) && ipip_allow != 2) {
> + if (!(ifp->if_flags & IFF_LOOPBACK) &&
> + atomic_load_int(&ipip_allow) != 2) {
> struct sockaddr_storage ss;
> struct rtentry *rt;
>
> @@ -584,19 +591,14 @@ int
> ipip_sysctl(int *name, u_int namelen, void *oldp, size_t *oldlenp, void *newp,
> size_t newlen)
> {
> - int error;
> -
> /* All sysctl names at this level are terminal. */
> if (namelen != 1)
> return (ENOTDIR);
>
> switch (name[0]) {
> case IPIPCTL_ALLOW:
> - NET_LOCK();
> - error = sysctl_int_bounded(oldp, oldlenp, newp, newlen,
> - &ipip_allow, 0, 2);
> - NET_UNLOCK();
> - return (error);
> + return (sysctl_int_bounded(oldp, oldlenp, newp, newlen,
> + &ipip_allow, 0, 2));
> case IPIPCTL_STATS:
> return (ipip_sysctl_ipipstat(oldp, oldlenp, newp));
> default:
sysctl(2): unlock ipip_sysctl()