syzbot reported an issue where pointer to ip header content was not
reloaded after xfrm_parse_spi().

Its not intuitive that this function changes skb->head, so switch to
skb_pointer_header.

Reported-by: syzbot <syzkal...@googlegroups.com>
Signed-off-by: Florian Westphal <f...@strlen.de>
---
 syzkaller reproducer did not spew a warning even after 15 minutes
 whereas before it would fire reliably after at most 30 seconds.

 include/net/xfrm.h    |  3 +--
 net/xfrm/xfrm_input.c | 39 +++++++++++++++++++++++----------------
 2 files changed, 24 insertions(+), 18 deletions(-)

diff --git a/include/net/xfrm.h b/include/net/xfrm.h
index 3cb618bbcfa5..07ffa9688a11 100644
--- a/include/net/xfrm.h
+++ b/include/net/xfrm.h
@@ -1579,7 +1579,6 @@ int xfrm4_rcv_encap(struct sk_buff *skb, int nexthdr, 
__be32 spi,
                    int encap_type);
 int xfrm4_transport_finish(struct sk_buff *skb, int async);
 int xfrm4_rcv(struct sk_buff *skb);
-int xfrm_parse_spi(struct sk_buff *skb, u8 nexthdr, __be32 *spi, __be32 *seq);
 
 static inline int xfrm4_rcv_spi(struct sk_buff *skb, int nexthdr, __be32 spi)
 {
@@ -1694,7 +1693,7 @@ int km_report(struct net *net, u8 proto, struct 
xfrm_selector *sel,
              xfrm_address_t *addr);
 
 void xfrm_input_init(void);
-int xfrm_parse_spi(struct sk_buff *skb, u8 nexthdr, __be32 *spi, __be32 *seq);
+int xfrm_parse_spi(const struct sk_buff *skb, u8 nexthdr, __be32 *spi, __be32 
*seq);
 
 void xfrm_probe_algs(void);
 int xfrm_count_pfkey_auth_supported(void);
diff --git a/net/xfrm/xfrm_input.c b/net/xfrm/xfrm_input.c
index 8ac9d32fb79d..92fbb784ea8c 100644
--- a/net/xfrm/xfrm_input.c
+++ b/net/xfrm/xfrm_input.c
@@ -141,37 +141,44 @@ EXPORT_SYMBOL(secpath_set);
 
 /* Fetch spi and seq from ipsec header */
 
-int xfrm_parse_spi(struct sk_buff *skb, u8 nexthdr, __be32 *spi, __be32 *seq)
+int xfrm_parse_spi(const struct sk_buff *skb, u8 nexthdr, __be32 *spi, __be32 
*seq)
 {
-       int offset, offset_seq;
-       int hlen;
+       int th = skb_transport_offset(skb);
+       union {
+               struct ip_auth_hdr _auth;
+               struct ip_comp_hdr _comp;
+               struct ip_esp_hdr _esp;
+       } _tmp;
+       const struct ip_auth_hdr *auth;
+       const struct ip_comp_hdr *comp;
+       const struct ip_esp_hdr *esp;
 
        switch (nexthdr) {
        case IPPROTO_AH:
-               hlen = sizeof(struct ip_auth_hdr);
-               offset = offsetof(struct ip_auth_hdr, spi);
-               offset_seq = offsetof(struct ip_auth_hdr, seq_no);
+               auth = skb_header_pointer(skb, th, sizeof(*auth), &_tmp._auth);
+               if (!auth)
+                       return -EINVAL;
+               *spi = auth->spi;
+               *seq = auth->seq_no;
                break;
        case IPPROTO_ESP:
-               hlen = sizeof(struct ip_esp_hdr);
-               offset = offsetof(struct ip_esp_hdr, spi);
-               offset_seq = offsetof(struct ip_esp_hdr, seq_no);
+               esp = skb_header_pointer(skb, th, sizeof(*esp), &_tmp._esp);
+               if (!esp)
+                       return -EINVAL;
+               *spi = esp->spi;
+               *seq = esp->seq_no;
                break;
        case IPPROTO_COMP:
-               if (!pskb_may_pull(skb, sizeof(struct ip_comp_hdr)))
+               comp = skb_header_pointer(skb, th, sizeof(*comp), &_tmp._comp);
+               if (!comp)
                        return -EINVAL;
-               *spi = htonl(ntohs(*(__be16 *)(skb_transport_header(skb) + 2)));
+               *spi = htonl(ntohs(comp->cpi));
                *seq = 0;
                return 0;
        default:
                return 1;
        }
 
-       if (!pskb_may_pull(skb, hlen))
-               return -EINVAL;
-
-       *spi = *(__be32 *)(skb_transport_header(skb) + offset);
-       *seq = *(__be32 *)(skb_transport_header(skb) + offset_seq);
        return 0;
 }
 EXPORT_SYMBOL(xfrm_parse_spi);
-- 
2.13.6

Reply via email to