Skip to content

Commit 8742dc8

Browse files
committed
xfrm4: Fix uninitialized memory read in _decode_session4
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: 1da177e ("Linux-2.6.12-rc2") Signed-off-by: Steffen Klassert <[email protected]>
1 parent 025c65e commit 8742dc8

File tree

1 file changed

+13
-11
lines changed

1 file changed

+13
-11
lines changed

net/ipv4/xfrm4_policy.c

Lines changed: 13 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -111,7 +111,8 @@ static void
111111
_decode_session4(struct sk_buff *skb, struct flowi *fl, int reverse)
112112
{
113113
const struct iphdr *iph = ip_hdr(skb);
114-
u8 *xprth = skb_network_header(skb) + iph->ihl * 4;
114+
int ihl = iph->ihl;
115+
u8 *xprth = skb_network_header(skb) + ihl * 4;
115116
struct flowi4 *fl4 = &fl->u.ip4;
116117
int oif = 0;
117118

@@ -122,6 +123,11 @@ _decode_session4(struct sk_buff *skb, struct flowi *fl, int reverse)
122123
fl4->flowi4_mark = skb->mark;
123124
fl4->flowi4_oif = reverse ? skb->skb_iif : oif;
124125

126+
fl4->flowi4_proto = iph->protocol;
127+
fl4->daddr = reverse ? iph->saddr : iph->daddr;
128+
fl4->saddr = reverse ? iph->daddr : iph->saddr;
129+
fl4->flowi4_tos = iph->tos;
130+
125131
if (!ip_is_fragment(iph)) {
126132
switch (iph->protocol) {
127133
case IPPROTO_UDP:
@@ -133,7 +139,7 @@ _decode_session4(struct sk_buff *skb, struct flowi *fl, int reverse)
133139
pskb_may_pull(skb, xprth + 4 - skb->data)) {
134140
__be16 *ports;
135141

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

139145
fl4->fl4_sport = ports[!!reverse];
@@ -146,7 +152,7 @@ _decode_session4(struct sk_buff *skb, struct flowi *fl, int reverse)
146152
pskb_may_pull(skb, xprth + 2 - skb->data)) {
147153
u8 *icmp;
148154

149-
xprth = skb_network_header(skb) + iph->ihl * 4;
155+
xprth = skb_network_header(skb) + ihl * 4;
150156
icmp = xprth;
151157

152158
fl4->fl4_icmp_type = icmp[0];
@@ -159,7 +165,7 @@ _decode_session4(struct sk_buff *skb, struct flowi *fl, int reverse)
159165
pskb_may_pull(skb, xprth + 4 - skb->data)) {
160166
__be32 *ehdr;
161167

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

165171
fl4->fl4_ipsec_spi = ehdr[0];
@@ -171,7 +177,7 @@ _decode_session4(struct sk_buff *skb, struct flowi *fl, int reverse)
171177
pskb_may_pull(skb, xprth + 8 - skb->data)) {
172178
__be32 *ah_hdr;
173179

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

177183
fl4->fl4_ipsec_spi = ah_hdr[1];
@@ -183,7 +189,7 @@ _decode_session4(struct sk_buff *skb, struct flowi *fl, int reverse)
183189
pskb_may_pull(skb, xprth + 4 - skb->data)) {
184190
__be16 *ipcomp_hdr;
185191

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

189195
fl4->fl4_ipsec_spi = htonl(ntohs(ipcomp_hdr[1]));
@@ -196,7 +202,7 @@ _decode_session4(struct sk_buff *skb, struct flowi *fl, int reverse)
196202
__be16 *greflags;
197203
__be32 *gre_hdr;
198204

199-
xprth = skb_network_header(skb) + iph->ihl * 4;
205+
xprth = skb_network_header(skb) + ihl * 4;
200206
greflags = (__be16 *)xprth;
201207
gre_hdr = (__be32 *)xprth;
202208

@@ -213,10 +219,6 @@ _decode_session4(struct sk_buff *skb, struct flowi *fl, int reverse)
213219
break;
214220
}
215221
}
216-
fl4->flowi4_proto = iph->protocol;
217-
fl4->daddr = reverse ? iph->saddr : iph->daddr;
218-
fl4->saddr = reverse ? iph->daddr : iph->saddr;
219-
fl4->flowi4_tos = iph->tos;
220222
}
221223

222224
static void xfrm4_update_pmtu(struct dst_entry *dst, struct sock *sk,

0 commit comments

Comments
 (0)