linux-kernel - Re: [PATCH] kunit: alloc_string_stream

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] [day] [month] [year] [list]

Message-ID: <CAGS_qxqWcD6wsncncqtFR00QO1QcLPm3e1Vzjto3ah-NoE3+jg@mail.gmail.com>
Date:   Mon, 31 Oct 2022 13:47:41 -0700
From:   Daniel Latypov <dlatypov@...gle.com>
To:     David Gow <davidgow@...gle.com>
Cc:     "YoungJun.park" <her0gyugyu@...il.com>,
        Brendan Higgins <brendan.higgins@...ux.dev>,
        linux-kselftest@...r.kernel.org, kunit-dev@...glegroups.com,
        linux-kernel@...r.kernel.org
Subject: Re: [PATCH] kunit: alloc_string_stream_fragment error handling bug fix

On Sat, Oct 29, 2022 at 8:20 PM 'David Gow' via KUnit Development
<kunit-dev@...glegroups.com> wrote:
>
> On Fri, Oct 28, 2022 at 10:43 PM YoungJun.park <her0gyugyu@...il.com> wrote:
> >
> > When it fails to allocate fragment, it does not free and return error.
> > And check the pointer inappropriately.
> >
> > Signed-off-by: YoungJun.park <her0gyugyu@...il.com>
> > ---
>
> Thanks! As Maíra points out, the added kunit_kfree() call isn't
> strictly necessary, though it definitely doesn't hurt (and it's
> probably a nice thing to free memory early if we're already in a
> pretty dire memory situation). So I think it's an improvement.
>
> The IS_ERR check is definitely a fix, though.

Note: the IS_ERR check was fixed already in
https://patchwork.kernel.org/project/linux-kselftest/patch/Y0kt1aCTHO4r2CmL@kili/
That change has made its way into torvalds/master.
So we could rebase this patch and reword it to talk just about the
early kfree().

Re free memory early:
It'll save us sizeof(struct string_stream_fragment) + sizeof(struct
kunit_resource), i.e. 24 + 56 = 80 bytes (on UML/x86_64).
So it's not much, but I guess it could help in edge cases.

Daniel