xen/tools: Fix symbols segfaults
diff mbox series

Message ID 20190403075602.43400-1-wipawel@amazon.de
State New, archived
Headers show
Series
  • xen/tools: Fix symbols segfaults
Related show

Commit Message

Wieczorkiewicz, Pawel April 3, 2019, 7:56 a.m. UTC
The symbols tool is outdated and has a bug in it leading to crashes.
The tool is derived from linux kernel where this bug has been already
fixed.

Original linux kernel commit:
e0a04b11e4059cab033469617 scripts/kallsyms.c: fix potential segfault

Signed-off-by: Pawel Wieczorkiewicz <wipawel@amazon.de>
Reviewed-by: Bjoern Doebel <doebel@amazon.de>
Reviewed-by: Norbert Manthey <nmanthey@amazon.de>
---
 xen/tools/symbols.c | 2 ++
 1 file changed, 2 insertions(+)

Comments

Jan Beulich April 3, 2019, 8:10 a.m. UTC | #1
>>> On 03.04.19 at 09:56, <wipawel@amazon.de> wrote:
> The symbols tool is outdated and has a bug in it leading to crashes.
> The tool is derived from linux kernel where this bug has been already
> fixed.

Thanks for noticing this omission of ours.

> Original linux kernel commit:
> e0a04b11e4059cab033469617 scripts/kallsyms.c: fix potential segfault
> 
> Signed-off-by: Pawel Wieczorkiewicz <wipawel@amazon.de>
> Reviewed-by: Bjoern Doebel <doebel@amazon.de>
> Reviewed-by: Norbert Manthey <nmanthey@amazon.de>

When we pull in changes (almost) verbatim from Linux, we typically
retain original authorship as well as the (possibly massaged)
title and description. See xen/arch/x86/cpu/mwait-idle.c's
history for some examples. I'll do this transformation before
committing the change, but in the future I'd appreciate if ported
patches were submitted that way.

Acked-by: Jan Beulich <jbeulich@suse.com>

I'm btw also confused by the Cc list you've used: You should
have Cc-ed THE REST, not just the tool stack maintainers.

Jan
Wieczorkiewicz, Pawel April 3, 2019, 9:07 a.m. UTC | #2
On 3. Apr 2019, at 10:10, Jan Beulich <JBeulich@suse.com<mailto:JBeulich@suse.com>> wrote:

On 03.04.19 at 09:56, <wipawel@amazon.de<mailto:wipawel@amazon.de>> wrote:
The symbols tool is outdated and has a bug in it leading to crashes.
The tool is derived from linux kernel where this bug has been already
fixed.

Thanks for noticing this omission of ours.

Anytime.


Original linux kernel commit:
e0a04b11e4059cab033469617 scripts/kallsyms.c: fix potential segfault

Signed-off-by: Pawel Wieczorkiewicz <wipawel@amazon.de<mailto:wipawel@amazon.de>>
Reviewed-by: Bjoern Doebel <doebel@amazon.de<mailto:doebel@amazon.de>>
Reviewed-by: Norbert Manthey <nmanthey@amazon.de<mailto:nmanthey@amazon.de>>

When we pull in changes (almost) verbatim from Linux, we typically
retain original authorship as well as the (possibly massaged)
title and description. See xen/arch/x86/cpu/mwait-idle.c's
history for some examples. I'll do this transformation before
committing the change, but in the future I'd appreciate if ported
patches were submitted that way.

Sure, I will be submitting patches that way.
It might make sense to add this explanation into the CONTRIBUTING
file to avoid this problem in the future (unless it documented already
somewhere and I simply missed it).


Acked-by: Jan Beulich <jbeulich@suse.com<mailto:jbeulich@suse.com>>

I'm btw also confused by the Cc list you've used: You should
have Cc-ed THE REST, not just the tool stack maintainers.

You’re right. My bad. Sorry.


Jan



Best Regards,
Pawel Wieczorkiewicz






Amazon Development Center Germany GmbH
Krausenstr. 38
10117 Berlin
Geschaeftsfuehrer: Christian Schlaeger, Ralf Herbrich
Ust-ID: DE 289 237 879
Eingetragen am Amtsgericht Charlottenburg HRB 149173 B
<html>
<head>
<meta http-equiv="Content-Type" content="text/html; charset=utf-8">
</head>
<body style="word-wrap: break-word; -webkit-nbsp-mode: space; line-break: after-white-space;" class="">
<br class="">
<div><br class="">
<blockquote type="cite" class="">
<div class="">On 3. Apr 2019, at 10:10, Jan Beulich &lt;<a href="mailto:JBeulich@suse.com" class="">JBeulich@suse.com</a>&gt; wrote:</div>
<br class="Apple-interchange-newline">
<div class="">
<div class="">
<blockquote type="cite" class="">
<blockquote type="cite" class="">
<blockquote type="cite" class="">On 03.04.19 at 09:56, &lt;<a href="mailto:wipawel@amazon.de" class="">wipawel@amazon.de</a>&gt; wrote:<br class="">
</blockquote>
</blockquote>
The symbols tool is outdated and has a bug in it leading to crashes.<br class="">
The tool is derived from linux kernel where this bug has been already<br class="">
fixed.<br class="">
</blockquote>
<br class="">
Thanks for noticing this omission of ours.<br class="">
</div>
</div>
</blockquote>
<div><br class="">
</div>
<div>Anytime.</div>
<br class="">
<blockquote type="cite" class="">
<div class="">
<div class=""><br class="">
<blockquote type="cite" class="">Original linux kernel commit:<br class="">
e0a04b11e4059cab033469617 scripts/kallsyms.c: fix potential segfault<br class="">
<br class="">
Signed-off-by: Pawel Wieczorkiewicz &lt;<a href="mailto:wipawel@amazon.de" class="">wipawel@amazon.de</a>&gt;<br class="">
Reviewed-by: Bjoern Doebel &lt;<a href="mailto:doebel@amazon.de" class="">doebel@amazon.de</a>&gt;<br class="">
Reviewed-by: Norbert Manthey &lt;<a href="mailto:nmanthey@amazon.de" class="">nmanthey@amazon.de</a>&gt;<br class="">
</blockquote>
<br class="">
When we pull in changes (almost) verbatim from Linux, we typically<br class="">
retain original authorship as well as the (possibly massaged)<br class="">
title and description. See xen/arch/x86/cpu/mwait-idle.c's<br class="">
history for some examples. I'll do this transformation before<br class="">
committing the change, but in the future I'd appreciate if ported<br class="">
patches were submitted that way.<br class="">
</div>
</div>
</blockquote>
<div><br class="">
</div>
<div>Sure, I will be submitting patches that way.</div>
<div>It might make sense to add this explanation into the CONTRIBUTING</div>
<div>file to avoid this problem in the future (unless it documented already</div>
<div>somewhere and I simply missed it).</div>
<br class="">
<blockquote type="cite" class="">
<div class="">
<div class=""><br class="">
Acked-by: Jan Beulich &lt;<a href="mailto:jbeulich@suse.com" class="">jbeulich@suse.com</a>&gt;<br class="">
<br class="">
I'm btw also confused by the Cc list you've used: You should<br class="">
have Cc-ed THE REST, not just the tool stack maintainers.<br class="">
</div>
</div>
</blockquote>
<div><br class="">
</div>
<div>You’re right. My bad. Sorry.</div>
<br class="">
<blockquote type="cite" class="">
<div class="">
<div class=""><br class="">
Jan<br class="">
<br class="">
<br class="">
</div>
</div>
</blockquote>
</div>
<br class="">
<div class="">
<div dir="auto" style="word-wrap: break-word; -webkit-nbsp-mode: space; line-break: after-white-space;" class="">
<div style="caret-color: rgb(0, 0, 0); color: rgb(0, 0, 0); font-family: Helvetica; font-size: 12px; font-style: normal; font-variant-caps: normal; font-weight: normal; letter-spacing: normal; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; word-spacing: 0px; -webkit-text-stroke-width: 0px; text-decoration: none;">
Best Regards,<br class="">
Pawel Wieczorkiewicz</div>
<br class="Apple-interchange-newline">
</div>
<br class="Apple-interchange-newline">
</div>
<br class="">
<br><br><br>Amazon Development Center Germany GmbH
<br>Krausenstr. 38
<br>10117 Berlin
<br>Geschaeftsfuehrer: Christian Schlaeger, Ralf Herbrich
<br>Ust-ID: DE 289 237 879
<br>Eingetragen am Amtsgericht Charlottenburg HRB 149173 B
<br><br><br>
</body>
</html>
Wei Liu April 3, 2019, 9:24 a.m. UTC | #3
On Wed, Apr 03, 2019 at 09:07:50AM +0000, Wieczorkiewicz, Pawel wrote:
> I'm btw also confused by the Cc list you've used: You should
> have Cc-ed THE REST, not just the tool stack maintainers.
> 
> You’re right. My bad. Sorry.
> 

./scripts/get_maintainers.pl is your friend. :-)

Wei.

Patch
diff mbox series

diff --git a/xen/tools/symbols.c b/xen/tools/symbols.c
index 8c5842d43f..05139d1600 100644
--- a/xen/tools/symbols.c
+++ b/xen/tools/symbols.c
@@ -525,6 +525,8 @@  static void optimize_result(void)
 
 			/* find the token with the breates profit value */
 			best = find_best_token();
+			if (token_profit[best] == 0)
+			        break;
 
 			/* place it in the "best" table */
 			best_table_len[i] = 2;