The dangers of static buffers in rsync code
jw schultz
jw at pegasys.ws
Tue Dec 30 10:01:35 EST 2003
On Mon, Dec 29, 2003 at 04:55:19PM -0500, Cristian Gafton wrote:
>
> I have been trying for quite a while now to understand why is the
> flist.c:f_name() function implemented using static buffers. Anyone care to
> comment?
>
> The immediate problem is that any call to f_name overrides the previous
> content (well, obvious). This, combined with the fact that several
> function calls are made with the result of f_name(file) results in
> problems handling hardlinks - and possibly other issues as well, but
> hardlinks is what made me look deeper inside the rsync code.
>
> As a side note, the f_name function has some "protection" against this
> static clobbering: instead of having a single buffer, it has... TEN, and
> it cycles through them. That's both funny and sad...
>
> One obvious problem path is starting from generator.c:generate_files()
> function. In there we call in a while loop:
>
> recv_generator(local_name?local_name:f_name(file),flist,i,f), which calls
> hlink.c:check_hard_link(), which calls
> hlink.c:hlink_compare(), which calls
> flist.c:file_compare(), which calls
> flist.c:f_name().
>
> Interested parties might want to follow the code, it is easier and more
> obvious...
>
> Anyway, the end result is that in recv_generator() the first argument
> (fname) is now clobbered by our search through the hlink_list, and
> depending on what was the last thing we put in there there are various
> failures happening.
>
> The bad part is that they are mostly silent. Most of the bitching is done
> via "if (verbose > 1)", so with the standard 'rsync -av' there are no
> indicators that would tell somebody rsync failed to handle a particular
> file...
>
> One way of "unclobbering" this is by forcing another call to f_name after
> the check_hard_link() call is finished (wither in check_hard_link itself
> or in recv_generator), but that smells rather nasty... On the other hand,
> reimplementing f_name to make it sane would require some bigger changes,
> since f_name() happens to be quite a popular function inside the rsync
> code...
>
> This is bad and needs some fixing... Little hammer or big hammer?
How about an even smaller hammer.
In recv_generator do a strlcpy of arg1 to an automatic.
--
________________________________________________________________
J.W. Schultz Pegasystems Technologies
email address: jw at pegasys.ws
Remember Cernan and Schmitt
More information about the rsync
mailing list