Re: [PATCH] Fix preserve_mode when destination directory partially exists

[Jim Meyering]

Jan Blunck <address@hidden> wrote:
> On Tue, Jan 08, Paul Eggert wrote:
>
>> Jim Meyering <address@hidden> writes:
>>
>> > 2008-01-07  Jan Blunck  <address@hidden>
>> >
>> >    cp --parents: don't use uninitialized memory when restoring permissions
>>
>> In reviewing that patch I noticed that the bug of using uninitialized
>> memory still remains in some (unlikely) cases.  If 'stat (src,
>> &new->st)' fails, the resulting uninitialized new->st buffer still
>> remains in the list of directories whose modes need fixing later.  As
>> far as I can tell the bug is triggered only in a race condition, where
>> a directory is moved as we are trying to copy it, so it's hard to come
>> up with a test case for it.  However, here's a patch.
>
> Hmm, is the stat() fails make_dir_parents_private() fails and nothing gets
> copied.
>
>> 2008-01-08  Paul Eggert  <address@hidden>
>>
>>      Fix a minor race condition when using cp -p --parents.
>>      * src/cp.c (make_dir_parents_private): If stat fails on the parent
>>      directory, do not add it to the list of directories whose modes
>>      might need fixing later.  Also, do not bother invoking 'stat'
>>      unless the stat results might be needed later.
>
> Yes, this is what I ment with my question what is expected when we call -p
> --parents and parts for the destination path is existing. Do we want to reset
> the modes of the existing directories or not? Your patch isn't preserving the
> modes for the existing directories in the destination path. This is a change
> to what was happening before your other patch I referenced in my first mail.

Thanks for looking at it.

My intent with cp -p --parents has been to preserve/propagate
permissions even to preexisting destination directories.
As far as I can see, it still does that.[*]

Can you demonstrate a failure?

Jim

[*] I've just realized that cp doesn't save/propagate SELinux context in
the same manner.  It probably should, in some cases, but I won't even
look at that until after 6.10.