netdev.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Stephen Hemminger <shemminger@osdl.org>
To: Alexey Kuznetsov <kuznet@ms2.inr.ac.ru>
Cc: "David S. Miller" <davem@davemloft.net>, netdev@vger.kernel.org
Subject: Re: [PATCH] ip multicast route bug fix
Date: Tue, 25 Jul 2006 08:55:28 -0700	[thread overview]
Message-ID: <20060725085528.38b86319@localhost.localdomain> (raw)
In-Reply-To: <20060725162001.GA15058@ms2.inr.ac.ru>

On Tue, 25 Jul 2006 20:20:01 +0400
Alexey Kuznetsov <kuznet@ms2.inr.ac.ru> wrote:

> Hello!
> 
> > Code was reusing an skb which could lead to use after free or double free.
> 
> No, this does not help. The bug is not here.
> 
> I was so ashamed of this that could not touch the thing. :-)
> It startled me a lot, how is it possible that the thing was in production
> for several years and such bad bug never was noticed?
> 
> Now it is clear. skbs leaked sometimes before BK changeset 1.889.26.53,
> subj: [IPV4]: Fix skb leak in inet_rtm_getroute. But after this fix,
> which introduced new bug (goto out_free in the enclosed patch),
> the bug showed on surface.
> 
> Please, review this.
> 
> - ipmr_cache_unresolved() does not free skb. Caller does.
> - goto out_free in route.c in the case, when skb is enqueued
>   is returned to goto out.
> - some cosmetic cleanup in rt_fill_info() to make it more readable.

This looks correct, but it may lead to false reports from automated
checking tools because the skb lifetime depends on the return value.
Wouldn't it be better to have a consistent interface (skb always freed),
and clone the skb if needed for deferred processing?

  reply	other threads:[~2006-07-25 16:36 UTC|newest]

Thread overview: 14+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2006-07-19 21:57 [PATCH] ip multicast route bug fix Stephen Hemminger
2006-07-25  5:29 ` James Morris
2006-07-25  6:03 ` Martin Josefsson
2006-07-25  8:03 ` Herbert Xu
2006-07-25 15:42   ` [PATCH] ip multicast route bug fix (rev2) Stephen Hemminger
2006-07-25 16:20 ` [PATCH] ip multicast route bug fix Alexey Kuznetsov
2006-07-25 15:55   ` Stephen Hemminger [this message]
2006-07-25 16:48     ` Alexey Kuznetsov
2006-07-25 18:31     ` Alexey Kuznetsov
2006-07-25 21:17     ` Alexey Kuznetsov
2006-07-25 21:24       ` Stephen Hemminger
2006-07-25 23:45       ` David Miller
2006-07-26  2:26       ` Herbert Xu
2006-07-26 13:27         ` Alexey Kuznetsov

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=20060725085528.38b86319@localhost.localdomain \
    --to=shemminger@osdl.org \
    --cc=davem@davemloft.net \
    --cc=kuznet@ms2.inr.ac.ru \
    --cc=netdev@vger.kernel.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).