linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Colin Ian King <colin.king@canonical.com>
To: Shaohua Li <shli@fb.com>
Cc: "David S. Miller" <davem@davemloft.net>,
	Hideaki YOSHIFUJI <yoshfuji@linux-ipv6.org>,
	David Ahern <dsahern@kernel.org>,
	Jakub Kicinski <kuba@kernel.org>,
	"netdev@vger.kernel.org" <netdev@vger.kernel.org>,
	"linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>
Subject: potential null pointer dereference in ip6_xmit
Date: Fri, 17 Sep 2021 17:41:53 +0100	[thread overview]
Message-ID: <516b6617-ab5e-4601-55e4-0f9844f9a49e@canonical.com> (raw)

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@fb.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

                 reply	other threads:[~2021-09-17 16:41 UTC|newest]

Thread overview: [no followups] expand[flat|nested]  mbox.gz  Atom feed

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=516b6617-ab5e-4601-55e4-0f9844f9a49e@canonical.com \
    --to=colin.king@canonical.com \
    --cc=davem@davemloft.net \
    --cc=dsahern@kernel.org \
    --cc=kuba@kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=netdev@vger.kernel.org \
    --cc=shli@fb.com \
    --cc=yoshfuji@linux-ipv6.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).