From: Anthony PERARD <anthony.perard@citrix.com>
To: Elliott Mitchell <ehem+xen@m5p.com>
Cc: <xen-devel@lists.xenproject.org>, Wei Liu <wl@xen.org>
Subject: Re: [PATCH 5/5] tools/xl: Fix potential deallocation bug
Date: Wed, 5 Jan 2022 15:05:23 +0000 [thread overview]
Message-ID: <YdWzswCszMN1IE20@perard> (raw)
In-Reply-To: <2d1335a4056558d172d9aa3e59982eb761647418.1640590794.git.ehem+xen@m5p.com>
On Thu, Dec 10, 2020 at 03:09:06PM -0800, Elliott Mitchell wrote:
> There is potential for the info and info_free variable's purposes to
> diverge. If info was overwritten with a distinct value, yet info_free
> still needed deallocation a bug would occur on this line. Preemptively
> address this issue (making use of divergent info/info_free values is
> under consideration).
>
> Signed-off-by: Elliott Mitchell <ehem+xen@m5p.com>
> ---
> tools/xl/xl_info.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/tools/xl/xl_info.c b/tools/xl/xl_info.c
> index 3647468420..938f06f1a8 100644
> --- a/tools/xl/xl_info.c
> +++ b/tools/xl/xl_info.c
> @@ -579,7 +579,7 @@ int main_list(int argc, char **argv)
> info, nb_domain);
>
> if (info_free)
> - libxl_dominfo_list_free(info, nb_domain);
> + libxl_dominfo_list_free(info_free, nb_domain);
>
> libxl_dominfo_dispose(&info_buf);
>
I don't think this is the right thing to do with this patch.
libxl_dominfo_list_free() should use the same variable that is used by
libxl_list_domain(). What we want to free is the allocation made by
libxl_list_domain().
"info_free" in the function seems to be used as a boolean which tell
if "info" have been allocated or not. Actually, it probably say if
"info" is a list of "libxl_dominfo" or not.
So instead of just replacing "info" by "info_free" here, we should
instead store the result from libxl_list_domain() into a different
variable and free that, like it is done with "info_buf".
I hope that makes sense?
Thanks,
--
Anthony PERARD
next prev parent reply other threads:[~2022-01-05 15:06 UTC|newest]
Thread overview: 15+ messages / expand[flat|nested] mbox.gz Atom feed top
2021-12-27 7:39 [PATCH 0/5] Some misc from my tree Elliott Mitchell
2020-12-10 23:09 ` [PATCH 5/5] tools/xl: Fix potential deallocation bug Elliott Mitchell
2021-12-29 17:20 ` Luca Fancellu
2022-01-05 15:05 ` Anthony PERARD [this message]
2022-04-22 22:58 ` Elliott Mitchell
2020-12-18 1:42 ` [PATCH 3/5] tools/xl: Rename printf_info()/list_domains_details() to dump_by_...() Elliott Mitchell
2021-12-29 17:18 ` Luca Fancellu
2020-12-18 1:42 ` [PATCH 4/5] tools/xl: Merge down debug/dry-run section of create_domain() Elliott Mitchell
2021-12-29 17:19 ` Luca Fancellu
2020-12-18 21:32 ` [PATCH 2/5] tools/xl: Mark libxl_domain_config * arg of printf_info_*() const Elliott Mitchell
2021-12-29 17:18 ` Luca Fancellu
2020-12-18 21:37 ` [PATCH 1/5] tools/libxl: Mark pointer args of many functions constant Elliott Mitchell
2021-12-29 17:15 ` Luca Fancellu
2022-01-05 10:09 ` Anthony PERARD
2022-01-05 13:09 ` Luca Fancellu
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=YdWzswCszMN1IE20@perard \
--to=anthony.perard@citrix.com \
--cc=ehem+xen@m5p.com \
--cc=wl@xen.org \
--cc=xen-devel@lists.xenproject.org \
/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
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.