Live-Patching Archive on lore.kernel.org
 help / color / Atom feed
From: Petr Mladek <pmladek@suse.com>
To: Yannick Cote <ycote@redhat.com>
Cc: live-patching@vger.kernel.org, linux-kselftest@vger.kernel.org,
	joe.lawrence@redhat.com
Subject: Re: [PATCH 4/4] selftests/livepatch: fix mem leaks in test-klp-shadow-vars
Date: Tue, 2 Jun 2020 11:57:50 +0200
Message-ID: <20200602095750.GK27273@linux-b0ei> (raw)
In-Reply-To: <20200528134849.7890-5-ycote@redhat.com>

On Thu 2020-05-28 09:48:49, Yannick Cote wrote:
> In some cases, when an error occurs during testing and the main test
> routine returns, a memory leak occurs via leaving previously registered
> shadow variables allocated in the kernel as well as shadow_ptr list
> elements. From now on, in case of error, remove all allocated shadow
> variables and shadow_ptr struct elements.
> 
> Signed-off-by: Yannick Cote <ycote@redhat.com>
> ---
>  lib/livepatch/test_klp_shadow_vars.c | 27 +++++++++++++++++++--------
>  1 file changed, 19 insertions(+), 8 deletions(-)
> 
> diff --git a/lib/livepatch/test_klp_shadow_vars.c b/lib/livepatch/test_klp_shadow_vars.c
> index 195309e1edf3..c6d631d826e0 100644
> --- a/lib/livepatch/test_klp_shadow_vars.c
> +++ b/lib/livepatch/test_klp_shadow_vars.c
> @@ -170,6 +170,7 @@ static int test_klp_shadow_vars_init(void)
>  	char *pndup[NUM_OBJS];
>  	int nfields2[NUM_OBJS], *pnfields2[NUM_OBJS], **sv2[NUM_OBJS];
>  	void **sv;
> +	int ret = -EINVAL;

IMHO, this predefined error value adds more hard than good. It makes
the code hard to read. One has to jump up and down to check what error
will get returned. Also it is safe only when "goto out" is used right
after setting this variable.


>  	int i;
>  
>  	ptr_id(NULL);
> @@ -192,12 +193,16 @@ static int test_klp_shadow_vars_init(void)
>  		/* alloc a few svars with different <obj> and <id>. */
>  		sv1[i] = shadow_alloc(&objs[i], SV_ID1, sizeof(pnfields1[i]),
>  					GFP_KERNEL, shadow_ctor, &pnfields1[i]);
> -		if (!sv1[i])
> -			return -ENOMEM;
> +		if (!sv1[i]) {
> +			ret = -ENOMEM;
> +			goto out;
> +		}
>  		sv2[i] = shadow_alloc(&objs[i], SV_ID2, sizeof(pnfields2[i]),
>  					GFP_KERNEL, shadow_ctor, &pnfields2[i]);
> -		if (!sv2[i])
> -			return -ENOMEM;
> +		if (!sv2[i]) {
> +			ret = -ENOMEM;
> +			goto out;
> +		}
>  	}
>  
>  	/* pass 2: verify we find allocated svars and where they point to */
> @@ -205,7 +210,7 @@ static int test_klp_shadow_vars_init(void)
>  		/* check the "char" svar for all objects */
>  		sv = shadow_get(&objs[i], SV_ID1);
>  		if (!sv)
> -			return -EINVAL;
> +			goto out;

Please, replace also return -EINVAL with

			ret = -EINVAL;
			goto out;


With this change:

Reviewed-by: Petr Mladek <pmladek@suse.com>

Best Regards,
Petr

  reply index

Thread overview: 14+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-05-28 13:48 [PATCH 0/4] selftests/livepatch: rework of test-klp-{callbacks,shadow_vars} Yannick Cote
2020-05-28 13:48 ` [PATCH 1/4] selftests/livepatch: rework test-klp-callbacks to use completion variables Yannick Cote
2020-06-02  8:16   ` Petr Mladek
2020-05-28 13:48 ` [PATCH 2/4] selftests/livepatch: rework test-klp-shadow-vars Yannick Cote
2020-06-02  9:25   ` Petr Mladek
2020-05-28 13:48 ` [PATCH 3/4] selftests/livepatch: more verification in test-klp-shadow-vars Yannick Cote
2020-06-01 11:27   ` Miroslav Benes
2020-06-01 11:39   ` Miroslav Benes
2020-06-02 12:35   ` Petr Mladek
2020-05-28 13:48 ` [PATCH 4/4] selftests/livepatch: fix mem leaks " Yannick Cote
2020-06-02  9:57   ` Petr Mladek [this message]
2020-05-29 15:12 ` [PATCH 0/4] selftests/livepatch: rework of test-klp-{callbacks,shadow_vars} Joe Lawrence
2020-06-01 11:48 ` Miroslav Benes
2020-06-02  5:01 ` Kamalesh Babulal

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20200602095750.GK27273@linux-b0ei \
    --to=pmladek@suse.com \
    --cc=joe.lawrence@redhat.com \
    --cc=linux-kselftest@vger.kernel.org \
    --cc=live-patching@vger.kernel.org \
    --cc=ycote@redhat.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link

Live-Patching Archive on lore.kernel.org

Archives are clonable:
	git clone --mirror https://lore.kernel.org/live-patching/0 live-patching/git/0.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 live-patching live-patching/ https://lore.kernel.org/live-patching \
		live-patching@vger.kernel.org
	public-inbox-index live-patching

Example config snippet for mirrors

Newsgroup available over NNTP:
	nntp://nntp.lore.kernel.org/org.kernel.vger.live-patching


AGPL code for this site: git clone https://public-inbox.org/public-inbox.git