GNOME Bugzilla – Bug 700507
Segfault comparing two files when accessing smb:///
Last modified: 2015-10-20 15:14:15 UTC
Hi, Using nautilus 3.8.1 from Ubuntu GNOME PPA, i get a crasher when browsing windows networks $ nautilus --version GNOME nautilus 3.8.1 Steps to reproduice : 1. open nautilus 2. Go to "Browse Networks" 3. Go to "Windows networks" 4. Wait a few seconds (it shows loading). Nautilus then segfaults. Here is the backtrace. Don't hesitate to ask me some more details. --------------------8<----------------------- Program received signal SIGSEGV, Segmentation fault. compare_by_display_name (file_1=0xab5980, file_2=0xab5b10) at nautilus-file.c:2907 2907 nautilus-file.c: Aucun fichier ou dossier de ce type. (gdb) bt
+ Trace 231956
$1 = (NautilusFile *) 0xab5980 (gdb) p nautilus_file_dump(file1) No symbol "file1" in current context. (gdb) p nautilus_file_dump(file_1) uri: smb:/// size: 0 kind: unknown $2 = void (gdb) p nautilus_file_dump(file_2) uri: smb:///NONSTOP size: 0 kind: unknown $3 = void (gdb) -------------------->8----------------------- Regards, Étienne BERSAC
I reproduced the bug with GNOME nautilus 3.6.3 : Same steps. --------------------8<----------------------- Program received signal SIGSEGV, Segmentation fault. compare_by_display_name (file_1=0xab17d0, file_2=0xab1320) at nautilus-file.c:2907 2907 nautilus-file.c: Aucun fichier ou dossier de ce type. (gdb) bt
+ Trace 231957
uri: smb:/// size: 0 kind: unknown [Thread 0x7fffd1895700 (LWP 3203) exited] $1 = void (gdb) p nautilus_file_dump(file_2) uri: smb:///NONSTOP size: 0 kind: unknown $2 = void (gdb) c Continuing. [Thread 0x7fffd3550700 (LWP 3185) exited] -------------------->8----------------------- Is this a bug with gvfs ? Regards
Thanks for the report and backtraces. They have some debug symbols missing (indicated by double question mark). Can you install glib debug package and retrieve the traces again? I guess the Ubuntu package is libglib2.0-0-dbg.
Can still reproduce this: ii nautilus 3.14.2-1
+ Trace 235305
Here name_1 = nautilus_file_peek_display_name (file_1); Return value is NULL, so there is a null dereference later in sort_last_1 = name_1[0] == SORT_LAST_CHAR1 || name_1[0] == SORT_LAST_CHAR2;
At work we have 1 domain and 1 workgoup. For some reason in linux a third workgroup/domain shows up but the display name is blank. Bypassing the compare_by_display_name fuction by just forcing it to return -1 stops the crash and allows our 2 "real" workgroups/domains to be browsed without any issue (there is a timeout when connecting to the blank one). The blank one has no name but when selected in nautilus shows "(null)" selected in the bottom right corner of the window. Kde shows 3 separate domains/workgroups as well including the blank one. This behavior is on nautilus 3.16.2
Downstream reports https://bugzilla.redhat.com/show_bug.cgi?id=1241679 https://bugs.launchpad.net/ubuntu/+source/nautilus/+bug/1297533 Backtrace from 3.16.2 "#0 compare_by_display_name (file_1=0x2b64c50, file_2=0x2b64df0) at nautilus-file.c:2881 name_1 = 0x0 name_2 = 0x2ac8a04 "ALBA" sort_last_1 = <optimized out> sort_last_2 = <optimized out> compare = <optimized out>
+ Trace 235563
The previous comment have a description of the issue in the code and seems to be due to smb entries with a blank name
https://bugs.launchpad.net/ubuntu/+source/nautilus/+bug/1280867 has some useful hint of on the cases leading to the issue "If one of the Ubuntu clients has a hostname that is too long to be automatically converted to a NetBIOS name, it shows up in the smbtree -N output with a blank NetBIOS name. Example: \\ really-long-name-of-computer server (Samba, Ubuntu) This blank name is what causes this bug in nautilus. If I go into the offending machines and give them a custom short NetBIOS name in /etc/samba/smb.conf, and restart nmbd on them, the problem goes away and I can browse the Windows network just fine. Perhaps compare_by_display_name() is not handling a NULL or empty string correctly? Either way, it looks like nmbd is auto-generating an empty name, so I would argue that it has a bug that is indirectly causing this bug at the same time."
The empty name seems to be a samba bug resolve upstream, see https://bugzilla.samba.org/show_bug.cgi?id=10896, it's probably going to take a while before the servers out there get a fixed samba so it would still be better if nautilus handling the empty name with segfaulting
Created attachment 313468 [details] [review] Handle nautilus_file_peek_display_name returning NULL
Review of attachment 313468 [details] [review]: peek_display_name already have checks for NULL display_names and it returns "" or it validates etc. What is not working there? And anyway, I think you just need to manage the special case there and not elsewhere in the code, because then any other client of peek_display_name will have the same non-obvious problem of display_name being NULL and got_custom_display_name being FALSE, which is against the invariant.
Created attachment 313666 [details] [review] nautilus_file_peek_display_name: Don't return NULL
There's a case where nautilus_file_set_display_name might not set the name, if it hits this. if (display_name == NULL || *display_name == 0) { return FALSE; } Are you asking for something like that? ^ (untested - I lost my way to reproduce this bug)
(In reply to Iain Lane from comment #12) > There's a case where nautilus_file_set_display_name might not set the name, > if it hits this. > > if (display_name == NULL || *display_name == 0) { > return FALSE; > } > ah right, just realized "" == *text == 0 .
Review of attachment 313666 [details] [review]: indeed, that was my suggestion. Can you do it even simpler?: return name ? eel_ref_str_peek (name) : ""; Also, can you provide a commit message explaining this issue?
> > return name ? eel_ref_str_peek (name) : ""; I meant: return file->details->display_name ? eel_ref_str_peek ( file->details->display_name) : "";
Created attachment 313675 [details] [review] nautilus_file_peek_display_name: Don't return NULL If the name is the empty string then nautilus_file_set_display_name won't actually set the display name. In this case we were returning NULL from nautilus_file_peek_display_name, which some of our callers weren't prepared to handle. This led to crashes.
Review of attachment 313675 [details] [review]: Perfect, thanks!
Attachment 313675 [details] pushed as 3fb7cff - nautilus_file_peek_display_name: Don't return NULL