lkml.org 
[lkml]   [2012]   [Feb]   [24]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    Date
    SubjectRe: RFC: memory leak in udp_table_init
    From
    On Fri, Feb 24, 2012 at 7:55 PM, Tim Bird <tim.bird@am.sony.com> wrote:
    > We've uncovered an obscure memory leak in the routine udp_table_init(),
    > in the file: net/ipv4/udp.c

    At a glance, I think what you are seeing is the same as this?

    http://lists.openwall.net/netdev/2011/06/22/12

    Paul.

    >
    > The allocation sequence is a bit weird, and I've got some questions
    > about the best way to fix it.
    >
    > Here's the code:
    >
    > void __init udp_table_init(struct udp_table *table, const char *name)
    > {
    >        unsigned int i;
    >
    >        if (!CONFIG_BASE_SMALL)
    >                table->hash = alloc_large_system_hash(name,
    >                        2 * sizeof(struct udp_hslot),
    >                        uhash_entries,
    >                        21, /* one slot per 2 MB */
    >                        0,
    >                        &table->log,
    >                        &table->mask,
    >                        64 * 1024);
    >        /*
    >         * Make sure hash table has the minimum size
    >         */
    >        if (CONFIG_BASE_SMALL || table->mask < UDP_HTABLE_SIZE_MIN - 1) {
    >                table->hash = kmalloc(UDP_HTABLE_SIZE_MIN *
    >                                      2 * sizeof(struct udp_hslot), GFP_KERNEL);
    >                if (!table->hash)
    >                        panic(name);
    >                table->log = ilog2(UDP_HTABLE_SIZE_MIN);
    >                table->mask = UDP_HTABLE_SIZE_MIN - 1;
    >        }
    >        ...
    >
    > We've seen instances where the second allocation of table->hash
    > is performed, wiping out the first hash table allocation, without a
    > free.  This ends up leaking the previously allocated hash table.
    >
    > That is, if we are !CONFIG_BASE_SMALL and for some reason
    > the alloc_large_system_hash() operation returns less than UDP_HTABLE_SIZE_MIN
    > hash slots, then it will trigger this.
    >
    > There's no complementary "free_large_system_hash()" which can be used to
    > back out of the first allocation, that I can find.
    >
    > We are currently doing the following to avoid the memory leak, but this
    > seems like it defeats the purpose of checking for the minimum size
    > (that is, if the first allocation was too small, we don't re-allocate).
    >
    > diff --git a/net/ipv4/udp.c b/net/ipv4/udp.c
    > index 5d075b5..2524af4 100644
    > --- a/net/ipv4/udp.c
    > +++ b/net/ipv4/udp.c
    > @@ -2194,7 +2194,8 @@ void __init udp_table_init(struct udp_table *table, const char *name)
    >        /*
    >         * Make sure hash table has the minimum size
    >         */
    > -       if (CONFIG_BASE_SMALL || table->mask < UDP_HTABLE_SIZE_MIN - 1) {
    > +       if ((CONFIG_BASE_SMALL || table->mask < UDP_HTABLE_SIZE_MIN - 1)
    > +               && !table->hash) {
    >                table->hash = kmalloc(UDP_HTABLE_SIZE_MIN *
    >                                      2 * sizeof(struct udp_hslot), GFP_KERNEL);
    >                if (!table->hash)
    >
    > Any suggestions for a way to correct for a too-small first allocation, without
    > a memory leak?
    >
    > Alternatively - how critical is this UDP_HTABLE_SIZE_MIN for correct operation
    > of the stack?
    >
    > Thanks for any information you can provide.
    >
    >  -- Tim
    >
    > =============================
    > Tim Bird
    > Architecture Group Chair, CE Workgroup of the Linux Foundation
    > Senior Staff Engineer, Sony Network Entertainment
    > =============================
    >
    > --
    > To unsubscribe from this list: send the line "unsubscribe netdev" in
    > the body of a message to majordomo@vger.kernel.org
    > More majordomo info at  http://vger.kernel.org/majordomo-info.html
    --
    To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
    the body of a message to majordomo@vger.kernel.org
    More majordomo info at http://vger.kernel.org/majordomo-info.html
    Please read the FAQ at http://www.tux.org/lkml/

    \
     
     \ /
      Last update: 2012-02-25 02:29    [W:0.027 / U:33.816 seconds]
    ©2003-2016 Jasper Spaans. hosted at Digital OceanAdvertise on this site