Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Potentially patching wrong files during recovery #1338

Closed
schlomo opened this issue May 1, 2017 · 12 comments · Fixed by #2055
Closed

Potentially patching wrong files during recovery #1338

schlomo opened this issue May 1, 2017 · 12 comments · Fixed by #2055
Assignees
Labels
Milestone

Comments

@schlomo
Copy link
Member

schlomo commented May 1, 2017

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.

image

@jsmeix
Copy link
Member

jsmeix commented May 16, 2017

@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
Copy link
Member

jsmeix commented May 16, 2017

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
Copy link
Member

jsmeix commented May 16, 2017

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
Copy link
Member

gdha commented Jan 12, 2018

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

@gdha
Copy link
Member

gdha commented Jan 12, 2018

@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 jsmeix added enhancement Adaptions and new features cleanup and removed support / question labels Jan 12, 2018
@jsmeix jsmeix added this to the ReaR v2.4 milestone Jan 12, 2018
@jsmeix
Copy link
Member

jsmeix commented Jan 12, 2018

I will have a look - as time permits.

Offhandedly I think #1638
is different but somehow related (also about symlink weirdness).

@gdha
Copy link
Member

gdha commented May 10, 2018

@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 jsmeix modified the milestones: ReaR v2.4, ReaR v2.5 May 14, 2018
@jsmeix
Copy link
Member

jsmeix commented May 14, 2018

@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
Copy link
Member

jsmeix commented Feb 26, 2019

In #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 added a commit that referenced this issue Feb 28, 2019
…during_finalize_stage_issue1338

Skip patching absolute symlinks during finalize stage
(except in 320_migrate_network_configuration_files.sh
because that code it too obsure for me). That does not
actually fix #1338
but for now it should at least avoid patching wrong files.
Furthermore do no longer create udev rules in the recreated system
that have not been there. This way one can avoid that ReaR creates
udev rules that are created and maintained by systemd/udev like
/etc/udev/rules.d/70-persistent-net.rules when one excludes such
udev rules from being restored from the backup or by moving them
away via BACKUP_RESTORE_MOVE_AWAY_FILES, cf.
#2055 (comment)
Finally moved 020_confirm_finalize.sh to 520_confirm_finalize.sh
so that it is run after the finalize scripts that modify certain files
because the whole idea behind 520_confirm_finalize.sh is that
the user can finally adapt his restored files as he wants, cf.
#2055 (comment)
@jsmeix jsmeix modified the milestones: ReaR v2.5, ReaR v2.6 Feb 28, 2019
@jsmeix
Copy link
Member

jsmeix commented Feb 28, 2019

Via #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 jsmeix reopened this Feb 28, 2019
@jsmeix jsmeix modified the milestones: ReaR v2.6, ReaR v2.7 Apr 29, 2020
@jsmeix
Copy link
Member

jsmeix commented Apr 29, 2020

Via
78d7a51
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
Copy link

github-actions bot commented Jul 2, 2020

Stale issue message

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants