Subject: Re: [PATCH] ip multicast route bug fix

From: Stephen Hemminger <[email-masked]>
Date: Tue, 25 Jul 2006 08:55:28 -0700

On Tue, 25 Jul 2006 20:20:01 +0400
Alexey Kuznetsov <[email-masked]> 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?