[BRLTTY] Crash of BRLTTY in X session

[Sebastian Humenda]

Hi

Samuel Thibault schrieb am 26.11.2024,  1:03 +0100:
>Sebastian Humenda, le lun. 25 nov. 2024 13:12:23 +0100, a ecrit:
>> Alright. Before I could reproduce the crash in X, I got earlier crashes with
>> the sanitiser enabled. The first one (log no. 1) occured a few seconds after
>> starting BRLTTY. The 2nd when activating the English language profile using
>> ShiftLeft + ShiftRight + E.
>
>There is fishy code in endItem_files: it sets gl_pathv elements to
>NULL before calling globfree. It happens that for glibc it's fine, but
>address sanitizer is right in crashing because the gl_pathc elements
>are supposed to be non-NULL. I don't see why this is doing that, one is
>not supposed to assume how this is allocated, so setting it to NULL is
>either a memory leak, or potential mis-free later on.
>
>For the time being, you can try to exchange these two lines, which will
>avoid the issue in asan.
>
>    globfree(&files->glob);
>    files->glob.gl_pathc = 0;

I've tried the attached patch. I took the packaging of BRLTTY 6.7-8 (a few
weeks back). The crashes got more frequent. The output below occured shortly
after the start of BRLTTY. addr2line for the given address wasn't helpful,
even through brltty-dbgsym (from my own version). Do I need to tell addr2line
where to look for the symbols?

Excerpt from the log and add2line:
% addr2line -e /usr/bin/brltty 109d9b
menu.c:?

Thanks
Sebastian
-------------- next part --------------
--- a/Programs/menu.c
+++ b/Programs/menu.c
@@ -657,8 +657,8 @@
 #if defined(HAVE_GLOB)
   if (files->glob.gl_pathc) {
     for (int i=0; i<files->glob.gl_offs; i+=1) files->glob.gl_pathv[i] = NULL;
-    globfree(&files->glob);
     files->glob.gl_pathc = 0;
+    globfree(&files->glob);
   }
 #elif defined(__MINGW32__)
   if (files->names) {
@@ -734,7 +734,7 @@
     }
 
     if (pattern) {
-      files->pattern = pattern; 
+      files->pattern = pattern;
 
       if ((files->initial = *initial? ensureFileExtension(initial, extension): strdup(""))) {
         if ((files->current = strdup(files->initial))) {