On Tue, Feb 4, 2025 at 10:12 PM Qing Zhao <[email protected]> wrote:
>
> Hi,
>
> One of our big application segv in libgcc code while unwinding the stack.
>
> This is a random crash while the application throws a c++ exception and
> unwinds the stack. Incidents are random and never can be reproduced by any
> test case.
>
> The libgcc that is used is based on GCC 8.
>
> Fortunately, we got some information through the stack trace, and narrowed
> down the crash is in the routine "init_object" of libgcc/unwind-dw2-pde.c:
>
> 777 count = classify_object_over_fdes (ob, ob->u.single);
>
> when loading the 2nd parameter of the call to "classify_object_over_fdes".
> i.e, the address that is pointed by "ob->u.single" is invalid.
>
> And the outer caller we can track back is the following line 1066 of the
> routine
> "_Unwind_Find_FDE":
>
> 1060 /* Classify and search the objects we've not yet processed. */
> 1061 while ((ob = unseen_objects))
> 1062 {
> 1063 struct object **p;
> 1064
> 1065 unseen_objects = ob->next;
> 1066 f = search_object (ob, pc);
>
> Then we suspected that the construction of the static variable
> "unseen_objects"
> might have some bug.
>
> Then after studying the source code that constructs "unseen_objects" in
> libgcc/unwind-dw2-pde.c, I found an issue in the routines "__register_frame"
> and "__register_frame_table" as following:
>
> 127 void
> 128 __register_frame (void *begin)
> 129 {
> 130 struct object *ob;
> 131
> 132 /* If .eh_frame is empty, don't register at all. */
> 133 if (*(uword *) begin == 0)
> 134 return;
> 135
> 136 ob = malloc (sizeof (struct object));
> 137 __register_frame_info (begin, ob);
> 138 }
>
> 181 void
> 182 __register_frame_table (void *begin)
> 183 {
> 184 struct object *ob = malloc (sizeof (struct object));
> 185 __register_frame_info_table (begin, ob);
> 186 }
> 187
>
> In the above, one obvious issue in the source code is at line 136, line 137,
> and line 184 and line 185: the return value of call to "malloc" is not checked
> against NULL before it was passed as the second parameter of the follwoing
> call.
>
> This might cause unpredicted random behavior.
>
> I checked the latest trunk GCC libgcc, the same issue is there too.
>
> This patch is for the latest trunk GCC.
>
> Please let me know any comments on this?
I think I've seen this elsewhere -- the issue is the unwind register API does
not allow for failures but I also think calling abort() is bad.
Are you calling this from a JIT context or so? We're assuming that at program
start malloc() will not fail.
The proper solution is probably to add an alternate ABI which has a way to fail
during registration.
Richard.
> thanks.
>
> Qing
>
>
> =======================
>
> Fix two issues in libgcc/unwind-dw2-fde.c:
>
> A. The returned value of call to malloc is not checked against NULL.
> This might cause unpredicted behavior during stack unwinding.
>
> B. Check for null begin parameter (as well as pointer to null) in
> __register_frame_info_table_bases and __register_frame_table.
>
> libgcc/ChangeLog:
>
> * unwind-dw2-fde.c (__register_frame): Check the return value of call
> to malloc.
> (__register_frame_info_table_bases): Check for null begin parameter.
> (__register_frame_table): Check the return value of call to malloc,
> Check for null begin parameter.
> ---
> libgcc/unwind-dw2-fde.c | 12 ++++++++++++
> 1 file changed, 12 insertions(+)
>
> diff --git a/libgcc/unwind-dw2-fde.c b/libgcc/unwind-dw2-fde.c
> index cdfd3974c99..f0bc29d682a 100644
> --- a/libgcc/unwind-dw2-fde.c
> +++ b/libgcc/unwind-dw2-fde.c
> @@ -169,6 +169,8 @@ __register_frame (void *begin)
> return;
>
> ob = malloc (sizeof (struct object));
> + if (ob == NULL)
> + abort ();
> __register_frame_info (begin, ob);
> }
>
> @@ -180,6 +182,10 @@ void
> __register_frame_info_table_bases (void *begin, struct object *ob,
> void *tbase, void *dbase)
> {
> + /* If .eh_frame is empty, don't register at all. */
> + if ((const uword *) begin == 0 || *(const uword *) begin == 0)
> + return;
> +
> ob->pc_begin = (void *)-1;
> ob->tbase = tbase;
> ob->dbase = dbase;
> @@ -210,7 +216,13 @@ __register_frame_info_table (void *begin, struct object
> *ob)
> void
> __register_frame_table (void *begin)
> {
> + /* If .eh_frame is empty, don't register at all. */
> + if (*(uword *) begin == 0)
> + return;
> +
> struct object *ob = malloc (sizeof (struct object));
> + if (ob == NULL)
> + abort ();
> __register_frame_info_table (begin, ob);
> }
>
> --
> 2.31.1
>
