[PATCH] binutils: objcopy/strip: fix preserving dates in ar archives

[Dmitry V. Levin]

On Tue, Sep 28, 2021 at 01:09:55AM +0300, Gleb Fotengauer-Malinovskiy wrote:
> After commit 985e0264516 copy_archive function began to pass invalid values to
> the utimensat(2) function when it tries to preserve timestamps in ar archives:
> 
> $ gcc -c -xc /dev/null -o t.o
> $ ar rcsU t.a t.o
> $ strace --failed-only -e /time strip -p t.a
> utimensat(AT_FDCWD, "steICERy/t.o", [{tv_sec=140428413401793, tv_nsec=48} /* 4451969-04-09T19:36:33.000000048+0000 */, {tv_sec=1632773474, tv_nsec=94919810130032}], 0) = -1 EINVAL (Invalid argument)
> strip: steICERy/t.o: cannot set time: Invalid argument
> +++ exited with 0 +++
> $ strace --failed-only -e /time objcopy -p t.a t1.a
> utimensat(AT_FDCWD, "stk5ToKP/t.o", [{tv_sec=640, tv_nsec=48} /* 1970-01-01T00:10:40.000000048+0000 */, {tv_sec=0, tv_nsec=223338299408}], 0) = -1 EINVAL (Invalid argument)
> objcopy: stk5ToKP/t.o: cannot set time: Invalid argument
> +++ exited with 0 +++
> 
> This happens because the bfd_stat_arch_elt implementation for ar
> archives fills only the st_mtim.tv_sec part of the st_mtim timespec
> structure, but leaves the st_mtim.tv_nsec part and the whole st_atim timespec
> untouched leaving them uninitialized.
> This behavior did not lead to any problems before because only
> the st_atim.tv_sec were passed to the utime(2) function uninitialized,
> but it was applied to temporary file, whose access time doesn't affect
> archive contents later.
> 
> Fixes: 985e0264516 ("PR27725, better objcopy -p times")

Thanks for the analysis.

> 	PR binutils/28391
> 	* binutils/objcopy.c (copy_archive): Clear buf using memset.
> ---
>  binutils/objcopy.c | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/binutils/objcopy.c b/binutils/objcopy.c
> index a6c2e0dcc26..867aba32cd4 100644
> --- a/binutils/objcopy.c
> +++ b/binutils/objcopy.c
> @@ -3600,6 +3600,7 @@ copy_archive (bfd *ibfd, bfd *obfd, const char *output_target,
>  
>        if (preserve_dates)
>  	{
> +	  memset(buf, &buf, 0, sizeof(buf));
>  	  stat_status = bfd_stat_arch_elt (this_element, &buf);

Did you mean memset(&buf, 0, sizeof(buf)) ?


-- 
ldv