<div dir="ltr"><div dir="ltr">On Fri, Oct 27, 2023 at 1:12 PM HAGIO KAZUHITO(萩尾　一仁) <<a href="mailto:k-hagio-ab@nec.com">k-hagio-ab@nec.com</a>> wrote: </div><div class="gmail_quote"><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">On 2023/10/27 12:44, lijiang wrote: > On Wed, Oct 25, 2023 at 4:09 PM HAGIO KAZUHITO(萩尾一仁) <<a href="mailto:k-hagio-ab@nec.com" target="_blank">k-hagio-ab@nec.com</a>> > wrote: > >> On 2023/10/24 18:10, Lianbo Jiang wrote: >>> This is a backport patch from gdb commit 58abdf887821 ("Verify COFF >>> symbol stringtab offset"). >>> >>> The AddressSanitizer reports a heap-use-after-free error as below: >>> gdb/coff-pe-read.c:137:27 >>> >>> Add a COFF offset check to fix this issue. >>> >>> Link: <a href="https://sourceware.org/bugzilla/show_bug.cgi?id=30640" rel="noreferrer" target="_blank">https://sourceware.org/bugzilla/show_bug.cgi?id=30640</a> >>> Signed-off-by: Lianbo Jiang <<a href="mailto:lijiang@redhat.com" target="_blank">lijiang@redhat.com</a>> >> >> Thank you for working on this issue. >> >>> --- >>> Please see the CVE-2023-39129. >>> >>> gdb-10.2.patch | 31 +++++++++++++++++++++++++++++++ >>> 1 file changed, 31 insertions(+) >>> >>> diff --git a/gdb-10.2.patch b/gdb-10.2.patch >>> index 7f4b86350bde..98538dc1138b 100644 >>> --- a/gdb-10.2.patch >>> +++ b/gdb-10.2.patch >>> @@ -3156,4 +3156,35 @@ exit 0 >>> + else if (i >= 0 && encoded[i] == '$') >>> len0 = i; >>> } >>> + >> >> No actual problem, but this empty '+' line is some strange... my working > > > Thank you for the comment, Kazu. > > After removing the empty "+" line, I got the following information during > compiling process. > ... > patching file gdb-10.2/gdb/coffread.c > Hunk #3 succeeded at 1318 with fuzz 1. yes, because there is need to shift a line like this: </blockquote><div>Good to me, thanks.</div><div> </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"> --- a/gdb-10.2.patch +++ b/gdb-10.2.patch @@ -3157,3 +3157,33 @@ exit 0 len0 = i; } +--- gdb-10.2/gdb/coffread.c.orig ++++ gdb-10.2/gdb/coffread.c > ... > > But, maybe we can ignore the above output. > > >> >> tree at a8e5e4cbae54 already has a space line at the end of file: >> >> $ tail -n 5 gdb-10.2.patch | tr ' ' '-' >> -------else-if-(encoded[i]-==-'$') >> +------else-if-(i->=-0-&&-encoded[i]-==-'$') >> ---------len0-=-i; >> -----} >> - >> >> but anyway, I can fix this while merging. >> >> Ok. > > >>> +--- gdb-10.2/gdb/coffread.c.orig >>> ++++ gdb-10.2/gdb/coffread.c >>> +@@ -159,6 +159,7 @@ static long linetab_offset; >>> + static unsigned long linetab_size; >>> + >>> + static char *stringtab = NULL; >>> ++static long stringtab_length = 0; >>> + >>> + extern void stabsread_clear_cache (void); >>> + >>> +@@ -1297,6 +1298,7 @@ init_stringtab (bfd *abfd, long offset, >> gdb::unique_xmalloc_ptr<char> *storage) >>> + /* This is in target format (probably not very useful, and not >>> + currently used), not host format. */ >>> + memcpy (stringtab, lengthbuf, sizeof lengthbuf); >>> ++ stringtab_length = length; >>> + if (length == sizeof length) /* Empty table -- just the count. >> */ >>> + return 0; >>> + >>> +@@ -1316,8 +1318,9 @@ getsymname (struct internal_syment *symbol_entry) >>> + >>> + if (symbol_entry->_n._n_n._n_zeroes == 0) >>> + { >>> +- /* FIXME: Probably should be detecting corrupt symbol files by >>> +- seeing whether offset points to within the stringtab. */ >>> ++ if (symbol_entry->_n._n_n._n_offset > stringtab_length) >>> ++ error (_("COFF Error: string table offset (%ld) outside string >> table (length %ld)"), >>> ++ symbol_entry->_n._n_n._n_offset, stringtab_length); >>> + result = stringtab + symbol_entry->_n._n_n._n_offset; >>> + } >>> + else >>> >> >> Just out of curiosity, do you have any information that crash can be > > > I did not reproduce it in crash-utility, and got the bug from the reporter. OK, thanks. > > >> >> affected by this issue? I don't know about the COFF, I wonder if the > > > Me too. > > embedded gdb does not use this, aside from an independent gdb.. >> >> > I just saw that the coff format was removed from gcc 8, so probably the gdb > does not use the coff format. > > I copied the following text from the link: > --- > Caveats > Support for the obsolete SDB/coff debug info format has been removed. The > option -gcoff no longer does anything. > ... > --- > Here is the link: <a href="https://gcc.gnu.org/gcc-8/changes.html" rel="noreferrer" target="_blank">https://gcc.gnu.org/gcc-8/changes.html</a> > > But I'm not sure if there are people who might compile crash-utility with > the old gcc( eg. < gcc 8), if no, we can drop this patch, and no need to > fix it. Any thoughts? I think that what compiles crash doesn't matter. Apparently the issue was reproduced by a crafted debuginfo. <a href="https://inbox.sourceware.org/gdb-patches/20230822152335.231921-1-keiths@redhat.com/T/#u" rel="noreferrer" target="_blank">https://inbox.sourceware.org/gdb-patches/20230822152335.231921-1-keiths@redhat.com/T/#u</a> Crash may not be affected with crash-specific checks, but it will be very hard to prove it. GDB maintainers decided to fix this, let's have it in the same way.. Hopefully it's removed in future :) So applied. <a href="https://github.com/crash-utility/crash/commit/fc6ed525407ffc6a181ab9b902f2fb23936309d2" rel="noreferrer" target="_blank">https://github.com/crash-utility/crash/commit/fc6ed525407ffc6a181ab9b902f2fb23936309d2</a> </blockquote><div> </div><div>Ok, thanks.</div><div> </div><div>Lianbo</div><div> </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"> Thanks, Kazu </blockquote></div></div>