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: <383a94bf-e9f1-49a9-9df9-1e72c51b5444@riseup.net>
Date:   Sat, 29 Oct 2022 07:35:30 -0300
From:   Maíra Canal <mairacanal@...eup.net>
To:     "YoungJun.park" <her0gyugyu@...il.com>,
        Brendan Higgins <brendan.higgins@...ux.dev>
Cc:     David Gow <davidgow@...gle.com>, 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 10/28/22 11:42, YoungJun.park 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>
> ---
>  lib/kunit/string-stream.c | 6 ++++--
>  1 file changed, 4 insertions(+), 2 deletions(-)
> 
> diff --git a/lib/kunit/string-stream.c b/lib/kunit/string-stream.c
> index 72659a9773e3..0228fe814e96 100644
> --- a/lib/kunit/string-stream.c
> +++ b/lib/kunit/string-stream.c
> @@ -23,8 +23,10 @@ static struct string_stream_fragment *alloc_string_stream_fragment(
>  		return ERR_PTR(-ENOMEM);
>  
>  	frag->fragment = kunit_kmalloc(test, len, gfp);
> -	if (!frag->fragment)
> +	if (!frag->fragment) {
> +		kunit_kfree(test, frag);

I don't believe that kunit_kfree is necessary here, because
kunit_kmalloc is like kmalloc, but the allocation is test managed, which
means that the allocation is managed by the test case and is
automatically cleaned up after the test case concludes.

So, the original version seems correct: if the allocation fails,
alloc_string_stream_fragment will return NULL and string_stream_vadd
will check if frag_container is different than NULL.

Best Regards,
- Maíra Canal

>  		return ERR_PTR(-ENOMEM);
> +	}
>  
>  	return frag;
>  }
> @@ -56,7 +58,7 @@ int string_stream_vadd(struct string_stream *stream,
>  	frag_container = alloc_string_stream_fragment(stream->test,
>  						      len,
>  						      stream->gfp);
> -	if (!frag_container)
> +	if (IS_ERR(frag_container))
>  		return -ENOMEM;
>  
>  	len = vsnprintf(frag_container->fragment, len, fmt, args);

Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ