xfrm4: Fix uninitialized memory read in _decode_session4 [Linux 3.16.72]

This Linux kernel change "xfrm4: Fix uninitialized memory read in _decode_session4" is included in the Linux 3.16.72 release. This change is authored by Steffen Klassert <steffen.klassert [at] secunet.com> on Tue Feb 26 07:04:50 2019 +0100. The commit for this change in Linux stable tree is c387702 (patch) which is from upstream commit 8742dc8. The same Linux upstream change may have been applied to various maintained Linux releases and you can find all Linux releases containing changes from upstream 8742dc8.

xfrm4: Fix uninitialized memory read in _decode_session4

commit 8742dc86d0c7a9628117a989c11f04a9b6b898f3 upstream.

We currently don't reload pointers pointing into skb header
after doing pskb_may_pull() in _decode_session4(). So in case
pskb_may_pull() changed the pointers, we read from random
memory. Fix this by putting all the needed infos on the
stack, so that we don't need to access the header pointers
after doing pskb_may_pull().

Fixes: 1da177e4c3f4 ("Linux-2.6.12-rc2")
Signed-off-by: Steffen Klassert <[email protected]>
Signed-off-by: Ben Hutchings <[email protected]>

There are 24 lines of Linux source code added/deleted in this change. Code changes to Linux kernel are as follows.

 net/ipv4/xfrm4_policy.c | 24 +++++++++++++-----------
 1 file changed, 13 insertions(+), 11 deletions(-)

diff --git a/net/ipv4/xfrm4_policy.c b/net/ipv4/xfrm4_policy.c
index 08e45c3..1b99e14 100644
--- a/net/ipv4/xfrm4_policy.c
+++ b/net/ipv4/xfrm4_policy.c
@@ -103,7 +103,8 @@ static int xfrm4_fill_dst(struct xfrm_dst *xdst, struct net_device *dev,
 _decode_session4(struct sk_buff *skb, struct flowi *fl, int reverse)
 {
    const struct iphdr *iph = ip_hdr(skb);
-   u8 *xprth = skb_network_header(skb) + iph->ihl * 4;
+   int ihl = iph->ihl;
+   u8 *xprth = skb_network_header(skb) + ihl * 4;
    struct flowi4 *fl4 = &fl->u.ip4;
    int oif = 0;

@@ -114,6 +115,11 @@ static int xfrm4_fill_dst(struct xfrm_dst *xdst, struct net_device *dev,
    fl4->flowi4_mark = skb->mark;
    fl4->flowi4_oif = reverse ? skb->skb_iif : oif;

+   fl4->flowi4_proto = iph->protocol;
+   fl4->daddr = reverse ? iph->saddr : iph->daddr;
+   fl4->saddr = reverse ? iph->daddr : iph->saddr;
+   fl4->flowi4_tos = iph->tos;
+
    if (!ip_is_fragment(iph)) {
        switch (iph->protocol) {
        case IPPROTO_UDP:
@@ -125,7 +131,7 @@ static int xfrm4_fill_dst(struct xfrm_dst *xdst, struct net_device *dev,
                pskb_may_pull(skb, xprth + 4 - skb->data)) {
                __be16 *ports;

-               xprth = skb_network_header(skb) + iph->ihl * 4;
+               xprth = skb_network_header(skb) + ihl * 4;
                ports = (__be16 *)xprth;

                fl4->fl4_sport = ports[!!reverse];
@@ -138,7 +144,7 @@ static int xfrm4_fill_dst(struct xfrm_dst *xdst, struct net_device *dev,
                pskb_may_pull(skb, xprth + 2 - skb->data)) {
                u8 *icmp;

-               xprth = skb_network_header(skb) + iph->ihl * 4;
+               xprth = skb_network_header(skb) + ihl * 4;
                icmp = xprth;

                fl4->fl4_icmp_type = icmp[0];
@@ -151,7 +157,7 @@ static int xfrm4_fill_dst(struct xfrm_dst *xdst, struct net_device *dev,
                pskb_may_pull(skb, xprth + 4 - skb->data)) {
                __be32 *ehdr;

-               xprth = skb_network_header(skb) + iph->ihl * 4;
+               xprth = skb_network_header(skb) + ihl * 4;
                ehdr = (__be32 *)xprth;

                fl4->fl4_ipsec_spi = ehdr[0];
@@ -163,7 +169,7 @@ static int xfrm4_fill_dst(struct xfrm_dst *xdst, struct net_device *dev,
                pskb_may_pull(skb, xprth + 8 - skb->data)) {
                __be32 *ah_hdr;

-               xprth = skb_network_header(skb) + iph->ihl * 4;
+               xprth = skb_network_header(skb) + ihl * 4;
                ah_hdr = (__be32 *)xprth;

                fl4->fl4_ipsec_spi = ah_hdr[1];
@@ -175,7 +181,7 @@ static int xfrm4_fill_dst(struct xfrm_dst *xdst, struct net_device *dev,
                pskb_may_pull(skb, xprth + 4 - skb->data)) {
                __be16 *ipcomp_hdr;

-               xprth = skb_network_header(skb) + iph->ihl * 4;
+               xprth = skb_network_header(skb) + ihl * 4;
                ipcomp_hdr = (__be16 *)xprth;

                fl4->fl4_ipsec_spi = htonl(ntohs(ipcomp_hdr[1]));
@@ -188,7 +194,7 @@ static int xfrm4_fill_dst(struct xfrm_dst *xdst, struct net_device *dev,
                __be16 *greflags;
                __be32 *gre_hdr;

-               xprth = skb_network_header(skb) + iph->ihl * 4;
+               xprth = skb_network_header(skb) + ihl * 4;
                greflags = (__be16 *)xprth;
                gre_hdr = (__be32 *)xprth;

@@ -205,10 +211,6 @@ static int xfrm4_fill_dst(struct xfrm_dst *xdst, struct net_device *dev,
            break;
        }
    }
-   fl4->flowi4_proto = iph->protocol;
-   fl4->daddr = reverse ? iph->saddr : iph->daddr;
-   fl4->saddr = reverse ? iph->daddr : iph->saddr;
-   fl4->flowi4_tos = iph->tos;
 }

 static inline int xfrm4_garbage_collect(struct dst_ops *ops)

Leave a Reply

Your email address will not be published. Required fields are marked *