pptp: Fix fib lookup calls.
authorGuillaume Nault <gnault@redhat.com>
Mon, 3 Jul 2023 17:14:46 +0000 (19:14 +0200)
committerDavid S. Miller <davem@davemloft.net>
Tue, 4 Jul 2023 18:39:30 +0000 (19:39 +0100)
PPTP uses pppox sockets (struct pppox_sock). These sockets don't embed
an inet_sock structure, so it's invalid to call inet_sk() on them.

Therefore, the ip_route_output_ports() call in pptp_connect() has two
problems:

  * The tos variable is set with RT_CONN_FLAGS(sk), which calls
    inet_sk() on the pppox socket.

  * ip_route_output_ports() tries to retrieve routing flags using
    inet_sk_flowi_flags(), which is also going to call inet_sk() on the
    pppox socket.

While PPTP doesn't use inet sockets, it's actually really layered on
top of IP and therefore needs a proper way to do fib lookups. So let's
define pptp_route_output() to get a struct rtable from a pptp socket.
Let's also replace the ip_route_output_ports() call of pptp_xmit() for
consistency.

In practice, this means that:

  * pptp_connect() sets ->flowi4_tos and ->flowi4_flags to zero instead
    of using bits of unrelated struct pppox_sock fields.

  * pptp_xmit() now respects ->sk_mark and ->sk_uid.

  * pptp_xmit() now calls the security_sk_classify_flow() security
    hook, thus allowing to set ->flowic_secid.

  * pptp_xmit() now passes the pppox socket to xfrm_lookup_route().

Found by code inspection.

Fixes: 00959ade36ac ("PPTP: PPP over IPv4 (Point-to-Point Tunneling Protocol)")
Signed-off-by: Guillaume Nault <gnault@redhat.com>
Signed-off-by: David S. Miller <davem@davemloft.net>
drivers/net/ppp/pptp.c

index 0fe7882..32183f2 100644 (file)
@@ -24,6 +24,7 @@
 #include <linux/in.h>
 #include <linux/ip.h>
 #include <linux/rcupdate.h>
+#include <linux/security.h>
 #include <linux/spinlock.h>
 
 #include <net/sock.h>
@@ -128,6 +129,23 @@ static void del_chan(struct pppox_sock *sock)
        spin_unlock(&chan_lock);
 }
 
+static struct rtable *pptp_route_output(struct pppox_sock *po,
+                                       struct flowi4 *fl4)
+{
+       struct sock *sk = &po->sk;
+       struct net *net;
+
+       net = sock_net(sk);
+       flowi4_init_output(fl4, sk->sk_bound_dev_if, sk->sk_mark, 0,
+                          RT_SCOPE_UNIVERSE, IPPROTO_GRE, 0,
+                          po->proto.pptp.dst_addr.sin_addr.s_addr,
+                          po->proto.pptp.src_addr.sin_addr.s_addr,
+                          0, 0, sock_net_uid(net, sk));
+       security_sk_classify_flow(sk, flowi4_to_flowi_common(fl4));
+
+       return ip_route_output_flow(net, fl4, sk);
+}
+
 static int pptp_xmit(struct ppp_channel *chan, struct sk_buff *skb)
 {
        struct sock *sk = (struct sock *) chan->private;
@@ -151,11 +169,7 @@ static int pptp_xmit(struct ppp_channel *chan, struct sk_buff *skb)
        if (sk_pppox(po)->sk_state & PPPOX_DEAD)
                goto tx_error;
 
-       rt = ip_route_output_ports(net, &fl4, NULL,
-                                  opt->dst_addr.sin_addr.s_addr,
-                                  opt->src_addr.sin_addr.s_addr,
-                                  0, 0, IPPROTO_GRE,
-                                  RT_TOS(0), sk->sk_bound_dev_if);
+       rt = pptp_route_output(po, &fl4);
        if (IS_ERR(rt))
                goto tx_error;
 
@@ -438,12 +452,7 @@ static int pptp_connect(struct socket *sock, struct sockaddr *uservaddr,
        po->chan.private = sk;
        po->chan.ops = &pptp_chan_ops;
 
-       rt = ip_route_output_ports(sock_net(sk), &fl4, sk,
-                                  opt->dst_addr.sin_addr.s_addr,
-                                  opt->src_addr.sin_addr.s_addr,
-                                  0, 0,
-                                  IPPROTO_GRE, RT_CONN_FLAGS(sk),
-                                  sk->sk_bound_dev_if);
+       rt = pptp_route_output(po, &fl4);
        if (IS_ERR(rt)) {
                error = -EHOSTUNREACH;
                goto end;