[plugins] Resolve symlink targets from real parent path

[srivathsa729]

Relative symlink targets must be resolved from the real directory that contains the symlink, not from the visible path used to reach it.

When a copied sysfs path contains symlinked parent components, resolving the relative target with dirname(srcpath) can produce invalid paths such as:

/sys/devices/system/cpu/devices/system/cpu/cpu0

Resolve the parent directory with realpath() before joining relative symlink targets. This avoids repeated failed stat messages from plugins that walk sysfs paths, including processor, scsi, md, and hardware.

Pablo Prado reported the issue and proposed this fix.

Reported-by: Pablo Prado pablo.prado@oracle.com

---

Please place an 'X' inside each '[]' to confirm you adhere to our Contributor Guidelines

• Is the commit message split over multiple lines and hard-wrapped at 72 characters?
• Is the subject and message clear and concise?
• Does the subject start with [plugin_name] if submitting a plugin patch or a [section_name] if part of the core sosreport code?
• Does the commit contain a Signed-off-by: First Lastname email@example.com?
• Are any related Issues or existing PRs properly referenced via a Closes (Issue) or Resolved (PR) line?
• Are all passwords or private data gathered by this PR obfuscated?

[packit-as-a-service]

Congratulations! One of the builds has completed. 🍾

You can install the built RPMs by following these steps:

• sudo dnf install -y 'dnf*-command(copr)'
• dnf copr enable packit/sosreport-sos-4352
• And now you can install the packages.

Please note that the RPMs should be used only in a testing environment.

[pmoravec]

I believe you have a valid issue and aiming to resolve it properly, but.. what I am doing wrong when trying to reproduce?

# cd /tmp/pr4352
# file $(find | sort)
.:                                       directory
./sys:                                   directory
./sys/bus:                               directory
./sys/bus/cpu:                           directory
./sys/bus/cpu/drivers:                   directory
./sys/bus/cpu/drivers/processor:         directory
./sys/bus/cpu/drivers/processor/cpu0:    symbolic link to ../../../../devices/system/cpu/cpu0
./sys/devices:                           directory
./sys/devices/system:                    directory
./sys/devices/system/cpu:                directory
./sys/devices/system/cpu/cpu0:           ASCII text
./sys/devices/system/cpu/cpu3:           directory
./sys/devices/system/cpu/cpu3/subsystem: symbolic link to ../../../../bus/cpu
#

Then modifying a plugin to collect that dir, I get collected the dir exactly as-is..?

(further, I smell there might be an issue/regression when collecting a symlink within a container (and within its sysroot) that targets outside the sysroot - I must think more about that use case)

[srivathsa729]

To reproduce you need to collect the nested symlink reached through the symlinked
parent, not only the symlinked directory itself.

The failing path is:

/tmp/pr4352/sys/devices/system/cpu/cpu3/subsystem/drivers/processor/cpu0

Here subsystem is a symlink to ../../../../bus/cpu, so the real parent of cpu0 is:

/tmp/pr4352/sys/bus/cpu/drivers/processor

The cpu0 symlink target is:

../../../../devices/system/cpu/cpu0

With the old code, that target is resolved from the visible parent path:

/tmp/pr4352/sys/devices/system/cpu/cpu3/subsystem/drivers/processor

which normalizes to the invalid path:

/tmp/pr4352/sys/devices/system/cpu/devices/system/cpu/cpu0

With this patch, the target is resolved from realpath(dirname(srcpath)), which
gives the correct path:

/tmp/pr4352/sys/devices/system/cpu/cpu0

So the issue is not with collecting cpu3/subsystem itself as a symlink. The issue
appears when sos later reaches symlinks below that path, such as subsystem/drivers/processor/cpu0.

On the container/sysroot case: yes, that is worth checking separately. My intent
here is only to change the base directory used for relative symlink targets from
the visible parent path to the real parent path, because the current behavior
creates invalid paths for sysfs layouts like the above.

[pmoravec]

Ah-a, finally I have a reproducer. With the above structure, let collect e.g. /tmp/pr4352/sys/devices (but not whole /tmp/pr4352/sys), like:

# echo ":/tmp/pr4352/sys/devices" > /etc/sos/extras.d/symlink.txt
# python3 bin/sos report -o sos_extras --batch --build
# cd $(ls -td /var/tmp/sos* 2>/dev/null | head -1)
# cd tmp/pr4352
# file $(find . | sort)
.:                                       directory
./sys:                                   directory
./sys/bus:                               directory
./sys/bus/cpu:                           directory
./sys/devices:                           directory
./sys/devices/system:                    directory
./sys/devices/system/cpu:                directory
./sys/devices/system/cpu/cpu0:           ASCII text
./sys/devices/system/cpu/cpu3:           directory
./sys/devices/system/cpu/cpu3/subsystem: symbolic link to ../../../../bus/cpu
#

tricky to identify the reproducer :) let me dig more..

[pmoravec]

Why not simply

realdir = os.path.realpath(os.path.dirname(srcpath))

as the srcdir is unused later on? (well, this is more clear code, so it does make a sense..)

[srivathsa729]

You are right, we can reduce these two lines to one line, but I kept it separated to make it easier to read, I can simply if you want.

[pmoravec]

The patch and unit test sounds good to me, should be idempotent to different sysroot in containers.