On Wed, Oct 25, 2023 at 4:09 PM HAGIO KAZUHITO(萩尾 一仁) <k-hagio-ab@xxxxxxx> 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: https://sourceware.org/bugzilla/show_bug.cgi?id=30640
> Signed-off-by: Lianbo Jiang <lijiang@xxxxxxxxxx>
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.
Hunk #3 succeeded at 1318 with fuzz 1.
...
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.
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.
Support for the obsolete SDB/coff debug info format has been removed. The option -gcoff no longer does anything.
...
---
Here is the link: https://gcc.gnu.org/gcc-8/changes.html
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?
Thanks.
Lianbo
Thanks,
Kazu
-- Crash-utility mailing list Crash-utility@xxxxxxxxxx https://listman.redhat.com/mailman/listinfo/crash-utility Contribution Guidelines: https://github.com/crash-utility/crash/wiki
