[<prev] [next>] [day] [month] [year] [list]
Message-ID: <516b6617-ab5e-4601-55e4-0f9844f9a49e@canonical.com>
Date: Fri, 17 Sep 2021 17:41:53 +0100
From: Colin Ian King <colin.king@...onical.com>
To: Shaohua Li <shli@...com>
Cc: "David S. Miller" <davem@...emloft.net>,
Hideaki YOSHIFUJI <yoshfuji@...ux-ipv6.org>,
David Ahern <dsahern@...nel.org>,
Jakub Kicinski <kuba@...nel.org>,
"netdev@...r.kernel.org" <netdev@...r.kernel.org>,
"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>
Subject: potential null pointer dereference in ip6_xmit
Hi,
Static analysis with Coverity detected a potential null pointer
deference in ip6_xmit, net/ipv6/ip6_output.c, I believe it may have been
introduced by the following commit:
commit 513674b5a2c9c7a67501506419da5c3c77ac6f08
Author: Shaohua Li <shli@...com>
Date: Wed Dec 20 12:10:21 2017 -0800
net: reevalulate autoflowlabel setting after sysctl setting
The analysis is as follows:
239 /*
240 * xmit an sk_buff (used by TCP, SCTP and DCCP)
241 * Note : socket lock is not held for SYNACK packets, but might be
modified
242 * by calls to skb_set_owner_w() and ipv6_local_error(),
243 * which are using proper atomic operations or spinlocks.
244 */
245 int ip6_xmit(const struct sock *sk, struct sk_buff *skb, struct
flowi6 *fl6,
246 __u32 mark, struct ipv6_txoptions *opt, int tclass, u32
priority)
247 {
248 struct net *net = sock_net(sk);
249 const struct ipv6_pinfo *np = inet6_sk(sk);
250 struct in6_addr *first_hop = &fl6->daddr;
251 struct dst_entry *dst = skb_dst(skb);
252 struct net_device *dev = dst->dev;
253 struct inet6_dev *idev = ip6_dst_idev(dst);
254 unsigned int head_room;
255 struct ipv6hdr *hdr;
256 u8 proto = fl6->flowi6_proto;
257 int seg_len = skb->len;
258 int hlimit = -1;
259 u32 mtu;
260
261 head_room = sizeof(struct ipv6hdr) + LL_RESERVED_SPACE(dev);
1. Condition opt, taking true branch.
262 if (opt)
263 head_room += opt->opt_nflen + opt->opt_flen;
264
2. Condition !!(head_room > skb_headroom(skb)), taking true branch.
265 if (unlikely(head_room > skb_headroom(skb))) {
266 skb = skb_expand_head(skb, head_room);
3. Condition !skb, taking false branch.
267 if (!skb) {
268 IP6_INC_STATS(net, idev,
IPSTATS_MIB_OUTDISCARDS);
269 return -ENOBUFS;
270 }
271 }
272
4. Condition opt, taking true branch.
273 if (opt) {
274 seg_len += opt->opt_nflen + opt->opt_flen;
275
5. Condition opt->opt_flen, taking true branch.
276 if (opt->opt_flen)
277 ipv6_push_frag_opts(skb, opt, &proto);
278
6. Condition opt->opt_nflen, taking true branch.
279 if (opt->opt_nflen)
280 ipv6_push_nfrag_opts(skb, opt, &proto,
&first_hop,
281 &fl6->saddr);
282 }
283
284 skb_push(skb, sizeof(struct ipv6hdr));
285 skb_reset_network_header(skb);
286 hdr = ipv6_hdr(skb);
287
288 /*
289 * Fill in the IPv6 header
290 */
7. Condition np, taking false branch.
8. var_compare_op: Comparing np to null implies that np might be null.
291 if (np)
292 hlimit = np->hop_limit;
9. Condition hlimit < 0, taking true branch.
293 if (hlimit < 0)
294 hlimit = ip6_dst_hoplimit(dst);
295
Dereference after null check (FORWARD_NULL)10. var_deref_model:
Passing null pointer np to ip6_autoflowlabel, which dereferences it.
296 ip6_flow_hdr(hdr, tclass, ip6_make_flowlabel(net, skb,
fl6->flowlabel,
297 ip6_autoflowlabel(net, np), fl6));
298
There is a null check on np on line 291, so potentially np could be null
on the call on line 296 where a null is passed to the function
ip6_autoflowlabel that dereferences the null np.
Colin
Powered by blists - more mailing lists