| Message ID | 20190403075602.43400-1-wipawel@amazon.de (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | xen/tools: Fix symbols segfaults | expand |
>>> 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
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 <<a href="mailto:JBeulich@suse.com" class="">JBeulich@suse.com</a>> 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, <<a href="mailto:wipawel@amazon.de" class="">wipawel@amazon.de</a>> 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 <<a href="mailto:wipawel@amazon.de" class="">wipawel@amazon.de</a>><br class=""> Reviewed-by: Bjoern Doebel <<a href="mailto:doebel@amazon.de" class="">doebel@amazon.de</a>><br class=""> Reviewed-by: Norbert Manthey <<a href="mailto:nmanthey@amazon.de" class="">nmanthey@amazon.de</a>><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 <<a href="mailto:jbeulich@suse.com" class="">jbeulich@suse.com</a>><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>
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.
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;