[Bug 1214249] New: rsync crashes when built with glibc-2.38
https://bugzilla.suse.com/show_bug.cgi?id=1214249 Bug ID: 1214249 Summary: rsync crashes when built with glibc-2.38 Classification: openSUSE Product: openSUSE Tumbleweed Version: Current Hardware: Other OS: Other Status: NEW Severity: Normal Priority: P5 - None Component: Basesystem Assignee: david.anes@suse.com Reporter: jslaby@suse.com QA Contact: qa-bugs@suse.de CC: pmonrealgonzalez@suse.com, schwab@suse.com Target Milestone: --- Found By: --- Blocker: --- rsync crashes in vim-plugins' build: https://build.opensuse.org/package/live_build_log/editors/vim-plugins/openSU... I believe it's due to glibc-2.38 update. If I update glibc to 2.38 only, rsync-3.2.7-3.1 does not crash. As soon as I update to rsync-3.2.7-3.2 (I believe the one rebuilt against this very new glibc), I see: $ rsync -FFXHav '--filter=merge global-rsync-filter' Align-37-43/ xxx sending incremental file list *** buffer overflow detected ***: terminated rsync: connection unexpectedly closed (0 bytes received so far) [Receiver] rsync error: error in rsync protocol data stream (code 12) at io.c(231) [Receiver=3.2.7] Neúspěšně ukončen (SIGABRT) (core dumped [obraz paměti uložen]) gdb says:
#3 0x00007f2a31226917 in __GI_abort () at abort.c:79 #4 0x00007f2a312277e3 in __libc_message (fmt=fmt@entry=0x7f2a313b030c "*** %s ***: terminated\n") at ../sysdeps/posix/libc_fatal.c:150 #5 0x00007f2a31327bdb in __GI___fortify_fail (msg=msg@entry=0x7f2a313b02f3 "buffer overflow detected") at fortify_fail.c:24 #6 0x00007f2a31327506 in __GI___chk_fail () at chk_fail.c:28 #7 0x00007f2a31329279 in __strlcpy_chk (s1=<optimized out>, s2=<optimized out>, n=<optimized out>, s1len=<optimized out>) at strlcpy_chk.c:28 27 if (__glibc_unlikely (s1len < n)) 28 __chk_fail (); #8 0x0000559d0acf778a in strlcpy (__n=4096, __src=0x7ffece39ae20 "xslaby/pokus/Align-37-43/", __dest=0x559d0ad61886 <dirbuf.lto_priv+6> "") at /usr/include/bits/string_fortified.h:156 156 return __strlcpy_chk (__dest, __src, __n, __glibc_objsize (__dest));
How does it come __glibc_objsize(dirbuf.lto_priv+6) is less than 4096?
#9 setup_merge_file (mergelist_num=mergelist_num@entry=0, ex=ex@entry=0x559d0bf84b40, lp=lp@entry=0x559d0bf84b90) at /usr/src/debug/rsync-3.2.7/exclude.c:737 737 strlcpy(y, save, MAXPATHLEN); #10 0x0000559d0acf7d94 in push_local_filters (dir=dir@entry=0x7ffece39c000 ".", dirlen=dirlen@entry=1) at /usr/src/debug/rsync-3.2.7/exclude.c:806 #11 0x0000559d0acf8259 in change_local_filter_dir (dname=0x7ffece39c000 ".", dlen=1, dir_depth=0) at /usr/src/debug/rsync-3.2.7/exclude.c:899 #12 0x0000559d0acef91c in send_file_list (f=4, argc=0, argv=0x559d0bf84898) at /usr/src/debug/rsync-3.2.7/flist.c:2453 #13 0x0000559d0ad07d4b in client_run (f_in=f_in@entry=5, f_out=f_out@entry=4, pid=pid@entry=6659, argc=argc@entry=1, argv=argv@entry=0x559d0bf84890) at /usr/src/debug/rsync-3.2.7/main.c:1315 #14 0x0000559d0ace2bdb in start_client (argv=0x559d0bf84890, argc=1) at /usr/src/debug/rsync-3.2.7/main.c:1613 #15 main (argc=<optimized out>, argv=<optimized out>) at /usr/src/debug/rsync-3.2.7/main.c:1873 -- You are receiving this mail because: You are on the CC list for the bug.
https://bugzilla.suse.com/show_bug.cgi?id=1214249 https://bugzilla.suse.com/show_bug.cgi?id=1214249#c1 Jiri Slaby <jslaby@suse.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Flags| |needinfo?(jh@suse.com) CC| |jh@suse.com --- Comment #1 from Jiri Slaby <jslaby@suse.com> --- (In reply to Jiri Slaby from comment #0)
rsync crashes in vim-plugins' build: https://build.opensuse.org/package/live_build_log/editors/vim-plugins/ openSUSE_Tumbleweed/x86_64
I believe it's due to glibc-2.38 update. If I update glibc to 2.38 only, rsync-3.2.7-3.1 does not crash.
As soon as I update to rsync-3.2.7-3.2 (I believe the one rebuilt against this very new glibc), I see: $ rsync -FFXHav '--filter=merge global-rsync-filter' Align-37-43/ xxx sending incremental file list *** buffer overflow detected ***: terminated rsync: connection unexpectedly closed (0 bytes received so far) [Receiver] rsync error: error in rsync protocol data stream (code 12) at io.c(231) [Receiver=3.2.7] Neúspěšně ukončen (SIGABRT) (core dumped [obraz paměti uložen])
gdb says:
#3 0x00007f2a31226917 in __GI_abort () at abort.c:79 #4 0x00007f2a312277e3 in __libc_message (fmt=fmt@entry=0x7f2a313b030c "*** %s ***: terminated\n") at ../sysdeps/posix/libc_fatal.c:150 #5 0x00007f2a31327bdb in __GI___fortify_fail (msg=msg@entry=0x7f2a313b02f3 "buffer overflow detected") at fortify_fail.c:24 #6 0x00007f2a31327506 in __GI___chk_fail () at chk_fail.c:28 #7 0x00007f2a31329279 in __strlcpy_chk (s1=<optimized out>, s2=<optimized out>, n=<optimized out>, s1len=<optimized out>) at strlcpy_chk.c:28 27 if (__glibc_unlikely (s1len < n)) 28 __chk_fail (); #8 0x0000559d0acf778a in strlcpy (__n=4096, __src=0x7ffece39ae20 "xslaby/pokus/Align-37-43/", __dest=0x559d0ad61886 <dirbuf.lto_priv+6> "") at /usr/include/bits/string_fortified.h:156 156 return __strlcpy_chk (__dest, __src, __n, __glibc_objsize (__dest));
How does it come __glibc_objsize(dirbuf.lto_priv+6) is less than 4096?
Provided it comes from parse_merge_name(): 602 static char buf[MAXPATHLEN]; ... 653 return buf; Is it an LTO fallout? Or gcc 13.2 fallout?
#9 setup_merge_file (mergelist_num=mergelist_num@entry=0, ex=ex@entry=0x559d0bf84b40, lp=lp@entry=0x559d0bf84b90) at /usr/src/debug/rsync-3.2.7/exclude.c:737 737 strlcpy(y, save, MAXPATHLEN); #10 0x0000559d0acf7d94 in push_local_filters (dir=dir@entry=0x7ffece39c000 ".", dirlen=dirlen@entry=1) at /usr/src/debug/rsync-3.2.7/exclude.c:806 #11 0x0000559d0acf8259 in change_local_filter_dir (dname=0x7ffece39c000 ".", dlen=1, dir_depth=0) at /usr/src/debug/rsync-3.2.7/exclude.c:899 #12 0x0000559d0acef91c in send_file_list (f=4, argc=0, argv=0x559d0bf84898) at /usr/src/debug/rsync-3.2.7/flist.c:2453 #13 0x0000559d0ad07d4b in client_run (f_in=f_in@entry=5, f_out=f_out@entry=4, pid=pid@entry=6659, argc=argc@entry=1, argv=argv@entry=0x559d0bf84890) at /usr/src/debug/rsync-3.2.7/main.c:1315 #14 0x0000559d0ace2bdb in start_client (argv=0x559d0bf84890, argc=1) at /usr/src/debug/rsync-3.2.7/main.c:1613 #15 main (argc=<optimized out>, argv=<optimized out>) at /usr/src/debug/rsync-3.2.7/main.c:1873 -- You are receiving this mail because: You are on the CC list for the bug.
https://bugzilla.suse.com/show_bug.cgi?id=1214249 https://bugzilla.suse.com/show_bug.cgi?id=1214249#c2 Jiri Slaby <jslaby@suse.com> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |rguenther@suse.com --- Comment #2 from Jiri Slaby <jslaby@suse.com> --- (In reply to Jiri Slaby from comment #1)
How does it come __glibc_objsize(dirbuf.lto_priv+6) is less than 4096?
Provided it comes from parse_merge_name(): 602 static char buf[MAXPATHLEN]; ... 653 return buf;
Is it an LTO fallout? Or gcc 13.2 fallout?
I cannot reproduce with 2 simple units: fun.c:1:#include <limits.h> fun.c:2: fun.c:3:__attribute__((noinline)) char *fun() fun.c:4:{ fun.c:5: static char buf[PATH_MAX]; fun.c:6: fun.c:7: return buf; fun.c:8:} fun.c:9: fun.h:1:#pragma once fun.h:2: fun.h:3:char *fun(); str.c:1:#include <err.h> str.c:2:#include <errno.h> str.c:3:#include <limits.h> str.c:4:#include <stdio.h> str.c:5:#include <stdlib.h> str.c:6:#include <string.h> str.c:7: str.c:8:#include "fun.h" str.c:9: str.c:10:int main(int, char **argv) str.c:11:{ str.c:12: char *buf = fun(); str.c:13: str.c:14: strlcpy(buf, argv[0], PATH_MAX); str.c:15: str.c:16: printf("%s\n", buf); str.c:17: str.c:18: return 0; str.c:19:} Maybe I would need to put them into 2 separated slices? LTO seems to propagate the constant (PATH_MAX) nicely to main and eliminates strlcpy_chk() completely above. -- You are receiving this mail because: You are on the CC list for the bug.
https://bugzilla.suse.com/show_bug.cgi?id=1214249 https://bugzilla.suse.com/show_bug.cgi?id=1214249#c4 --- Comment #4 from Jiri Slaby <jslaby@suse.com> --- (In reply to Dominique Leuenberger from comment #3)
(In reply to Jiri Slaby from comment #1)
Provided it comes from parse_merge_name(): 602 static char buf[MAXPATHLEN]; ... 653 return buf;
according to valgrind it comes from setup_merge_file (exclude.c:737)
Which obtains the buffer from parse_merge_name() above -- see line 693 -- if I am looking correctly? -- You are receiving this mail because: You are on the CC list for the bug.
https://bugzilla.suse.com/show_bug.cgi?id=1214249 https://bugzilla.suse.com/show_bug.cgi?id=1214249#c5 --- Comment #5 from Jiri Slaby <jslaby@suse.com> --- (In reply to Jiri Slaby from comment #4)
(In reply to Dominique Leuenberger from comment #3)
(In reply to Jiri Slaby from comment #1)
Provided it comes from parse_merge_name(): 602 static char buf[MAXPATHLEN]; ... 653 return buf;
according to valgrind it comes from setup_merge_file (exclude.c:737)
Which obtains the buffer from parse_merge_name() above -- see line 693 -- if I am looking correctly?
But you gave me a hint. I didn't study the code in between the crash and parse_merge_name() before. Now I have. Well, the buffer pointer might be incremented and that means the MAXPATHLEN in strlcpy(y, save, MAXPATHLEN); is not be correct in that case. I.e. 'y' might not equal 'buf' from the above and can point to the middle of 'buf', right? -- You are receiving this mail because: You are on the CC list for the bug.
https://bugzilla.suse.com/show_bug.cgi?id=1214249 https://bugzilla.suse.com/show_bug.cgi?id=1214249#c6 Richard Biener <rguenther@suse.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Flags|needinfo?(jh@suse.com) | --- Comment #6 from Richard Biener <rguenther@suse.com> --- Frame #9 points to while (*y) { char save[MAXPATHLEN]; strlcpy(save, y, MAXPATHLEN); *y = '\0'; dirbuf_len = y - dirbuf; strlcpy(x, ex->pattern, MAXPATHLEN - (x - buf)); parse_filter_file(lp, buf, ex, XFLG_ANCHORED2ABS); if (ex->rflags & FILTRULE_NO_INHERIT) { /* Free the undesired rules to clean up any per-dir * mergelists they defined. Otherwise pop_local_filters * may crash trying to restore nonexistent state for * those mergelists. */ free_filters(lp->head); lp->head = NULL; } lp->tail = NULL; strlcpy(y, save, MAXPATHLEN); ^^^ that would be an automatic var 'save'. We're using _FORTIFY_SOURCE=3 and thus __builtin_dynamic_object_size (__o, 1). Yes, 'y' is initially from parse_merge_name, either based on 'buf' or the incoming argument 'merge_file'. But 'y' is also incremented in a lot of places and while 'buf' is of size MAXPATAHLEN I don't see how it can reliably still have MAXPATHLEN space here. Did you check what s1len actually is? But yes, __builtin_dynamic_object size does have some bugs but none I am aware of for this specifically. It would be nice to know whether the issue reproduces when rsync is not built with -flto. Do you have an actual testcase? Doing what you quote fails due to missing global-rsync-filter (and missing source files, possibly not needed).
./rsync -FFXHav '--filter=merge global-rsync-filter' Align-37-43/ xxx rsync: [client] failed to open exclude file global-rsync-filter: No such file or directory (2) rsync error: error in file IO (code 11) at exclude.c(1481) [client=3.2.7]
I'll note the fortification will fail once the computed length is less than the advertised, thus when GCC figures out _anything_ and the code actually incremented 'p'. The code is definitely fragile, possibly not expecting fortification ... For example we have if (*x) y += strlen(y); /* nope -- skip the scan */ and since strlen is actually computed we now know symbolically the space left in 'y' (that it's bound by MAXPATHLEN minus strlen(y) at that point). That we can symbolically compute that means there's enough chance to run into this bug of rsync. IMHO the strlcpy(y, save, MAXPATHLEN); is simply not correct (without proving that GCC computes a correct value here, but that should be possible to verify debugging the testcase). -- You are receiving this mail because: You are on the CC list for the bug.
https://bugzilla.suse.com/show_bug.cgi?id=1214249 https://bugzilla.suse.com/show_bug.cgi?id=1214249#c7 --- Comment #7 from Richard Biener <rguenther@suse.com> --- I'll note again that __strlcpy_chk does if (__glibc_unlikely (s1len < n)) __chk_fail (); return __strlcpy (s1, s2, n); so it doesn't check whether 's2' might be shorter than s1len and thus the call is actually safe (it might be safe to call strnlen (s2, n)). -- You are receiving this mail because: You are on the CC list for the bug.
https://bugzilla.suse.com/show_bug.cgi?id=1214249 https://bugzilla.suse.com/show_bug.cgi?id=1214249#c9 Jiri Slaby <jslaby@suse.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Flags| |needinfo? See Also| |https://github.com/WayneD/r | |sync/issues/511 --- Comment #9 from Jiri Slaby <jslaby@suse.com> --- (In reply to Richard Biener from comment #6)
Did you check what s1len actually is?
Unfortunately "optimized out". It's held only in a register which was apparently overwritten in further frames.
Do you have an actual testcase? Doing what you quote fails due to missing global-rsync-filter (and missing source files, possibly not needed).
Sorry, it's: https://build.opensuse.org/package/view_file/editors/vim-plugins/global-rsyn...
IMHO the
strlcpy(y, save, MAXPATHLEN);
is simply not correct (without proving that GCC computes a correct value here, but that should be possible to verify debugging the testcase).
Yeah, agreed, I created: https://github.com/WayneD/rsync/issues/511 -- You are receiving this mail because: You are on the CC list for the bug.
https://bugzilla.suse.com/show_bug.cgi?id=1214249 https://bugzilla.suse.com/show_bug.cgi?id=1214249#c10 Jiri Slaby <jslaby@suse.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Flags|needinfo? | --- Comment #10 from Jiri Slaby <jslaby@suse.com> --- What about https://build.opensuse.org/request/show/1104626 ? -- You are receiving this mail because: You are on the CC list for the bug.
https://bugzilla.suse.com/show_bug.cgi?id=1214249 https://bugzilla.suse.com/show_bug.cgi?id=1214249#c11 Richard Biener <rguenther@suse.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Flags|needinfo? | --- Comment #11 from Richard Biener <rguenther@suse.com> --- (In reply to Jiri Slaby from comment #10)
What about https://build.opensuse.org/request/show/1104626 ?
I can see how this avoids triggering the fortification check when it will turn out harmless. Of course the underlying problem is still present. Note doing strlcpy (dest, src, strlen (src)) is effectively making the use of strlcpy pointless, so a more honest patch would have replaced it with strcpy (dest, src) instead ... -- You are receiving this mail because: You are on the CC list for the bug.
https://bugzilla.suse.com/show_bug.cgi?id=1214249 https://bugzilla.suse.com/show_bug.cgi?id=1214249#c12 Michael Matz <matz@suse.com> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |matz@suse.com --- Comment #12 from Michael Matz <matz@suse.com> --- The patch is wrong. The third argument to strlcpy is supposed to contain the size of the _destination_ buffer (pointer to that in first argument). As such it has no inherent relations to the source string at all. Clearly the author of that code wanted to prevent buffer overruns, otherwise he hadn't used strlcpy, so it's better to fix that for good. You could for instance use this pattern to do the right thing: char buf[MAXPATHLEN], *bufend = buf + MAXPATHLEN, *y = buf; ... stuff that manipulates y ... strlcpy (y, input, bufend - y); -- You are receiving this mail because: You are on the CC list for the bug.
https://bugzilla.suse.com/show_bug.cgi?id=1214249 https://bugzilla.suse.com/show_bug.cgi?id=1214249#c13 David Anes <david.anes@suse.com> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |opensuse@mike.franken.de --- Comment #13 from David Anes <david.anes@suse.com> --- *** Bug 1214616 has been marked as a duplicate of this bug. *** -- You are receiving this mail because: You are on the CC list for the bug.
https://bugzilla.suse.com/show_bug.cgi?id=1214249 https://bugzilla.suse.com/show_bug.cgi?id=1214249#c15 --- Comment #15 from David Anes <david.anes@suse.com> --- (In reply to Michael Matz from comment #12)
The patch is wrong. (...)
I was about to close this one as fixed, but then I saw your comment. I see a patch was already sent some time ago by Dirk (and sent along other fixes later by me) in the following request: https://build.opensuse.org/request/show/1109241 Is the current version of the patch the one that's wrong? Can you please review and adjust it if needed? Thanks. -- You are receiving this mail because: You are on the CC list for the bug.
https://bugzilla.suse.com/show_bug.cgi?id=1214249 https://bugzilla.suse.com/show_bug.cgi?id=1214249#c16 --- Comment #16 from Michael Matz <matz@suse.com> --- The patch from SR#1109241 looks fine at a cursory look, it tracks the bytes left in the destination buffer for the second strlcpy call. (FWIW, the initial patch in SR#1104626 was the one that was wrong, but it's now lost in the mist of time :) ) So, I think it's all fine now, regarding this bug report here. -- You are receiving this mail because: You are on the CC list for the bug.
https://bugzilla.suse.com/show_bug.cgi?id=1214249 https://bugzilla.suse.com/show_bug.cgi?id=1214249#c17 David Anes <david.anes@suse.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Resolution|--- |FIXED Status|NEW |RESOLVED --- Comment #17 from David Anes <david.anes@suse.com> --- (In reply to Michael Matz from comment #16)
The patch from SR#1109241 looks fine at a cursory look, it tracks the bytes left in the destination buffer for the second strlcpy call. (FWIW, the initial patch in SR#1104626 was the one that was wrong, but it's now lost in the mist of time :) ) So, I think it's all fine now, regarding this bug report here.
Awesome, thanks for such a quick reply. Therefore I'm closing this one a fixed. -- You are receiving this mail because: You are on the CC list for the bug.
participants (1)
-
bugzilla_noreply＠suse.com