#1338 Issue `closed`: Potentially patching wrong files during recovery¶

Labels: enhancement, cleanup, no-issue-activity

schlomo opened issue at 2017-05-01 20:02:¶

because somebody follows symlinks into the rescue system. I suppose that we should look into doing all sorts of file-based operations in the restored file space under chroot so that absolute symlinks will actually refer to the matching files in the restores file systems.

jsmeix commented at 2017-05-16 08:51:¶

@schlomo
I do not fully understand what exactly you mean but
I guess you are talking about the section

        # sed -i bails on symlinks, so we follow the symlink and patch the result
        # on dead links we warn and skip them
        # TODO: maybe we must put this into a chroot so that absolute symlinks will work correctly
        if test -L "$file" ; then
                if linkdest="$(readlink -f "$file")" ; then
                        # if link destination is residing on /proc we skip it silently
                        echo $linkdest | grep -q "^/proc" && continue
                        LogPrint "Patching '$linkdest' instead of '$file'"
                        file="$linkdest"
                else
                        LogPrint "Not patching dead link '$file'"
                        continue
                fi
        fi

in each of the
finalize/GNU/Linux/250_migrate_disk_devices_layout.sh
finalize/GNU/Linux/250_migrate_lun_wwid.sh
finalize/GNU/Linux/280_migrate_uuid_tags.sh
scripts?

jsmeix commented at 2017-05-16 09:04:¶

Interestingly there is another 'finalize' script
finalize/GNU/Linux/320_migrate_network_configuration_files.sh
that also runs 'sed -i' but does not care about symlinks:

        for network_file in $TARGET_FS_ROOT/etc/sysconfig/*/ifcfg-*${new_mac}* $TARGET_FS_ROOT/etc/sysconfig/*/ifcfg-*${dev}*; do
        ...
            sed -i -e "$SED_SCRIPT" "$network_file"

jsmeix commented at 2017-05-16 09:25:¶

On my SLES11 and SLES12 systems (I will not look at SLES10 ;-)
"man sed" shows:

       --follow-symlinks
              follow symlinks when processing in place

       -i[SUFFIX], --in-place[=SUFFIX]
              edit files in place (makes backup if SUFFIX supplied)

so that "sed --follow-symlinks -i file_or_symlink" should "just work"
which it does on my SLES12 system:

# echo old >foo

# ln -s foo slfoo

# ls -l *foo
-rw-r--r-- 1 root root 4 May 16 11:12 foo
lrwxrwxrwx 1 root root 3 May 16 11:12 slfoo -> foo

# sed --follow-symlinks -i -e 's/old/new/' slfoo

# echo $?
0

e205:~ # cat foo
new

BUT hooray!:-( the same "just fails" on my SLES11 system with

# ls -l *foo
-rw-r--r-- 1 root root 4 May 16 11:15 foo
lrwxrwxrwx 1 root root 3 May 16 11:09 slfoo -> foo

# sed --follow-symlinks -i -e 's/old/new/' slfoo
sed: ck_follow_symlink: couldn't lstat s/foo: No such file or directory

# echo $?
4

WTH blabbers it about 's/foo'?
It is left as an exercise to the reader to find some
matching bug reports about 'sed' on the Internet ;-)

gdha commented at 2018-01-12 08:21:¶

When we implement request from issue #1638 this may fix this?

gdha commented at 2018-01-12 08:23:¶

@jsmeix As you are owner of issue #1638 I dare to assign this issue to you as well - if you think this is not appropriate then un-assign yourself

jsmeix commented at 2018-01-12 13:06:¶

I will have a look - as time permits.

Offhandedly I think https://github.com/rear/rear/issues/1638
is different but somehow related (also about symlink weirdness).

gdha commented at 2018-05-10 08:38:¶

@jsmeix any update on this topic. It wouldn't hurt to move the milestone in my opinion as well if you do not have the time for it. It is not urgent at all. Thanks

jsmeix commented at 2018-05-14 07:48:¶

@gdha
unfortunately I found no time to have a closer look at this issue
but because we got no actualy issue reports in this area
I think we can move it to the next ReaR 2.5.

jsmeix commented at 2019-02-26 15:21:¶

In https://github.com/rear/rear/pull/2055
I tried to improve symlink handling in finalize scripts a bit
(avoid to patch wrong files as a first step)
but to me the finalize scripts look somewhat obscure
(it seems they "just do" some stuff without much care)
so that basically I feel somewhat overstrained with that task...

jsmeix commented at 2019-02-28 13:51:¶

Via https://github.com/rear/rear/pull/2055
a first step towards actually fixing this issue is done.

Fixing this issue is not doable in the currently planned time until the
ReaR 2.5 release so that I postpone it for the next ReaR 2.6 release, cf.
https://github.com/rear/rear/milestones

jsmeix commented at 2020-04-29 15:37:¶

Via
https://github.com/rear/rear/commit/78d7a519b4700c828544df00763a90886e0847a6
I introduced the valid_restored_file_for_patching function for now only in
finalize/GNU/Linux/320_migrate_network_configuration_files.sh
https://github.com/rear/rear/blob/master/usr/share/rear/finalize/GNU/Linux/320_migrate_network_configuration_files.sh#L47
to see if and how it works there.

If things work o.k. there I think the valid_restored_file_for_patching function
is the right generic way to determine the actualy right target system files.

github-actions commented at 2020-07-02 01:33:¶

Stale issue message

[Export of Github issue for rear/rear.]

#1338 Issue closed: Potentially patching wrong files during recovery¶