Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]


Groups > linux.kernel > #1618981

Re: net/ipv4: use-after-free in ipv4_mtu

From Cong Wang <xiyou.wangcong@gmail.com>
Newsgroups linux.kernel
Subject Re: net/ipv4: use-after-free in ipv4_mtu
Date 2017-04-07 19:20 +0200
Message-ID <ttFId-1OF-9@gated-at.bofh.it> (permalink)
References (1 earlier) <tsBQm-hL-27@gated-at.bofh.it> <tsHM5-4f4-7@gated-at.bofh.it> <tsJbb-54X-1@gated-at.bofh.it> <tt1KO-8w3-21@gated-at.bofh.it> <ttd9g-7m3-5@gated-at.bofh.it>
Organization linux.* mail to news gateway

Show all headers | View raw


On Thu, Apr 6, 2017 at 3:49 AM, Eric Dumazet <eric.dumazet@gmail.com> wrote:
> On Wed, 2017-04-05 at 15:33 -0700, Cong Wang wrote:
>
>> Good find! I missed the refcnt in rt_set_nexthop() before that commit.
>>
>> We need to revert that commit to restore the refcnt for fib_info.
>
> Well, there are other spots , in decnet and IPv6.

IPv6 is very different, it copies or steals the metrics from mx6_config:

static int fib6_commit_metrics(struct dst_entry *dst, struct mx6_config *mxc)
{
        if (!mxc->mx)
                return 0;

        if (dst->flags & DST_HOST) {
                u32 *mp = dst_metrics_write_ptr(dst);

                if (unlikely(!mp))
                        return -ENOMEM;

                fib6_copy_metrics(mp, mxc);
        } else {
                dst_init_metrics(dst, mxc->mx, false);

                /* We've stolen mx now. */
                mxc->mx = NULL;
        }

        return 0;
}

so probably doesn't need a refcnt.

Decnet has already done the refcnt'ing, see dn_fib_semantic_match().


>
> This is why my original mail stated the problem was in the calls to :
>
> dst_init_metrics(&rt->dst, fi->fib_metrics, true);
>
> Lets do not think in "reverting" spirit, but adding the missing bits.
>
> The problem here is that the metrics should not be freed until last user
> is gone.
>
> So maybe a refcount should be added to metrics, and we do not have to
> add a fib pointer again in all dsts.
>

Good point, but it is harder than just revert given that fact that dst metrics
is a magic pointer to an array and COW.

Back to linux.kernel | Previous | NextPrevious in thread | Find similar | Unroll thread


Thread

net/ipv4: use-after-free in ipv4_mtu Andrey Konovalov <andreyknvl@google.com> - 2017-04-04 17:00 +0200
  Re: net/ipv4: use-after-free in ipv4_mtu Eric Dumazet <edumazet@google.com> - 2017-04-04 21:00 +0200
    Re: net/ipv4: use-after-free in ipv4_mtu Cong Wang <xiyou.wangcong@gmail.com> - 2017-04-05 03:20 +0200
      Re: net/ipv4: use-after-free in ipv4_mtu Eric Dumazet <eric.dumazet@gmail.com> - 2017-04-05 04:50 +0200
        Re: net/ipv4: use-after-free in ipv4_mtu Subash Abhinov Kasiviswanathan <subashab@codeaurora.org> - 2017-04-05 21:00 +0200
        Re: net/ipv4: use-after-free in ipv4_mtu Cong Wang <xiyou.wangcong@gmail.com> - 2017-04-06 00:40 +0200
          Re: net/ipv4: use-after-free in ipv4_mtu Eric Dumazet <eric.dumazet@gmail.com> - 2017-04-06 12:50 +0200
            Re: net/ipv4: use-after-free in ipv4_mtu Cong Wang <xiyou.wangcong@gmail.com> - 2017-04-07 19:20 +0200

csiph-web