netdev - Re: genetlink: prevent memory leak in netlbl_unlabel

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <CAHC9VhRk8Gc_Yexrjz5uif+Vj7d+b=uMUytbrmbm2Yv+zoM05w@mail.gmail.com>
Date:   Fri, 27 Sep 2019 10:48:54 -0400
From:   Paul Moore <paul@...l-moore.com>
To:     Markus Elfring <Markus.Elfring@....de>
Cc:     Navid Emamdoost <navid.emamdoost@...il.com>,
        linux-security-module@...r.kernel.org, netdev@...r.kernel.org,
        Navid Emamdoost <emamd001@....edu>, Kangjie Lu <kjlu@....edu>,
        linux-kernel@...r.kernel.org, kernel-janitors@...r.kernel.org,
        Stephen A McCamant <smccaman@....edu>,
        "David S. Miller" <davem@...emloft.net>
Subject: Re: genetlink: prevent memory leak in netlbl_unlabel_defconf

On Fri, Sep 27, 2019 at 9:15 AM Markus Elfring <Markus.Elfring@....de> wrote:
>
> > > In netlbl_unlabel_defconf if netlbl_domhsh_add_default fails the
> > > allocated entry should be released.
> …
> > That said, netlbl_unlabel_defconf() *should* clean up here just on
> > principal if nothing else.
>
> How do you think about to add the tag “Fixes” then?

>From what I've seen the "Fixes" tag is typically used by people who
are backporting patches, e.g. the -stable folks, to help decide what
they need to backport.  As I mentioned in my previous email this
missing free doesn't actually manifest itself as a practical leak on
any of the existing kernels so there isn't a need to backport this
patch.  For that reason I would probably skip the "Fixes" metadata
here, but I don't feel strongly enough about it to object if others
want it.  FWIW, I play things very conservatively when talking about
backporting patches to stable kernels; if it doesn't fix a serious
user-visible bug it shouldn't be backported IMHO.

This patch is more of a conceptual fix than a practical fix.  Not that
there is anything wrong with this patch, I just think it isn't as
critical as most people would think from reading "memory leak" in the
subject line.  Yes, there is a memory leak, but the kernel panics soon
after so it's a bit moot.  Further, even if the panic was somehow
skipped (?) the memory leak only happens once during boot; the failed
initialization is undoubtedly going to be far more damaging to the
system than a few lost bytes of memory.

-- 
paul moore
www.paul-moore.com