Re: [PATCH] selinux: free str on error in str_read()

[Kees Cook <keescook@chromium.org>]

On Wed, Apr 15, 2020 at 06:04:53PM -0400, Paul Moore wrote:
> On Tue, Apr 14, 2020 at 10:25 AM Ondrej Mosnacek <omosnace@redhat.com> wrote:
> >
> > In [see "Fixes:"] I missed the fact that str_read() may give back an
> > allocated pointer even if it returns an error, causing a potential
> > memory leak in filename_trans_read_one(). Fix this by making the
> > function free the allocated string whenever it returns a non-zero value,
> > which also makes its behavior more obvious and prevents repeating the
> > same mistake in the future.
> >
> > Reported-by: coverity-bot <keescook+coverity-bot@chromium.org>
> > Addresses-Coverity-ID: 1461665 ("Resource leaks")
> > Fixes: c3a276111ea2 ("selinux: optimize storage of filename transitions")
> > Signed-off-by: Ondrej Mosnacek <omosnace@redhat.com>
> > ---
> >  security/selinux/ss/policydb.c | 8 ++++----
> >  1 file changed, 4 insertions(+), 4 deletions(-)
> 
> I just merged this into selinux/stable-5.7 and assuming all goes well
> in testing I'll send this up to Linus later this week.  Thanks Ondrej.
> 
> I also want to add my thanks to the "coverity bot", thanks Kees.  Are
> you only running this only on Linus tree?  If it's open to other trees
> it might be nice to get the selinux/next branch into the automated
> testing.

It's being run on linux-next. The free coverity scanner barely has the
resources is keep up with one tree, so I just feed it -next. They were
kind enough to let us upload daily now, so I've been trying to feed the
emailed reports back. It's all just the tip of the iceberg, of course.

-- 
Kees Cook