FWUPD affecting dru, causing null pointer deref in the kernel

[rishabh-chromium]

Describe the bug
Seems like the start v1.9.6 fwupd is affecting the DRU in a manner that it is causing a null pointer deref.
The null ptr dereference happens in amdgpu_dm_i2c_xfer (link)
ddc_service->link is NULL and is dereferenced passing arguments into dc_submit_i2c.

Steps to Reproduce

stop fwupd
/usr/local/libexec/igt-gpu-tools/amdgpu/amd_pci_unplug --> pass
start fwupd
/usr/local/libexec/igt-gpu-tools/amdgpu/amd_pci_unplug --> panic

Logs/Debugging
Found this log message in fwupd.log
failed to add device /sys/devices/pci0000:00/0000:00:08.1/0000:04:00.0/drm/card2/card2-eDP-2/drm_dp_aux0: failed to add device using on synaptics_mst: failed to open /dev/drm_dp_aux0: No such file or directory

Issue is temporarily solved if I add this line to fwupd.conf

DisabledPlugins=mediatek_scaler;

fwupd version information
1.9.6

fwupdmgr --version

[rishabh-chromium]

version:

compile   org.freedesktop.fwupd         1.9.6
compile   com.hughsie.libxmlb           0.3.11
compile   com.hughsie.libjcat           0.1.8
compile   org.freedesktop.gusb          0.4.5
runtime   org.freedesktop.gusb          0.4.5
runtime   org.kernel                    5.15.135-20720-g95a1928abe0e
runtime   org.freedesktop.fwupd         1.9.6

[superm1]

Any chance you can test this against main to see if it's still a problem?

@hughsie I guess we should move mediatek scalar probe to the same as synaptics mst does now too.

[rishabh-chromium]

I tested this today and spoke with Richard about this, definitely seems to be a bug here

[rishabh-chromium]

Want to mention that this has also been tested with the latest kernel and the issue persists

[superm1]

I tested this today and spoke with Richard about this, definitely seems to be a bug here

OK, so tested fwupd main you mean?

[superm1]

From your description I'd think this can help it from kernel side. Can you try this?

diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
index 868946dd7ef1..98c85bd9136a 100644
--- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
+++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
@@ -7433,7 +7433,7 @@ static int amdgpu_dm_i2c_xfer(struct i2c_adapter *i2c_adap,

        cmd.payloads = kcalloc(num, sizeof(struct i2c_payload), GFP_KERNEL);

-       if (!cmd.payloads)
+       if (!cmd.payloads || !ddc_service->link)
                return result;

        cmd.number_of_payloads = num;

[superm1]

failed to add device /sys/devices/pci0000:00/0000:00:08.1/0000:04:00.0/drm/card2/card2-eDP-2/drm_dp_aux0: failed to add device using on synaptics_mst: failed to open /dev/drm_dp_aux0: No such file or directory

Also; I'd like to better understand this device if you can share more about it. It's the "3rd" DRM device found; and "second" eDP of that device. Is it contrived as a result of IGT tests or is that real?

[rishabh-chromium]

Oh I am stupid, I didn't think you meant main branch. It seems like testing it with main solves it! We will CP that change!

Thank you for digging out the right CL! Will close the issue once I confirm it is fully solved

[superm1]

OK, I think there is still a kernel bug here though too. Let me post you one more patch. If you can please test this against your test case.

0001-drm-amd-display-fix-a-NULL-pointer-dereference-in-am.patch

[rishabh-chromium]

Do you want me to try out that patch against fwupd v1.9.6?

[superm1]

Yes; I think we have two bugs. One with fwupd how it was handling the DPCD probing/scan and one in the kernel.
As you have a good way to reproduce both if you can please use your failing test environment I think this is the correct patch for the kernel aspect of it. If you can confirm it works I'll submit it upstream and get it pulled back to stable. It looks like an unintentional regression introduced in kernel 6.0 with torvalds/linux@22676bc500c2.

[hughsie]

@hughsie I guess we should move mediatek scalar probe to the same as synaptics mst does now too.

I guess we could, but I don't know what that would achieve -- I don't think that scaler does anything with DPCD.

[superm1]

I guess we could, but I don't know what that would achieve -- I don't think that scaler does anything with DPCD.

Oh, my memory failed me how it works because of this reported issue and turning off Mediatek scaler plugin helped. Yeah, carry on.

[ddavenport-chromium]

With the patch posted here applied, I hit a panic due to a NULL pointer dereference because ddc_service->ddc_pin was NULL.