Comment # 6 on bug 1214249 from Richard Biener

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).