Created attachment 140263 [details] [review] Fix null pointer dereference in drmmode_output_dpms This fixes an Xorg crash when a Lenovo P50 laptop is undocked, disconnecting it from two external DisplayPort monitors. Checking for koutput == NULL has precedent in drmmode_output_get_modes.
Full backtrace: Thread 1 "Xorg" received signal SIGSEGV, Segmentation fault. drmmode_output_dpms (output=0x558b2dab8ee0, mode=3) at ../../src/drmmode_display.c:921 921 ../../src/drmmode_display.c: No such file or directory. #0 0x00007f386322a759 in drmmode_output_dpms (output=0x558b2dab8ee0, mode=3) at ../../src/drmmode_display.c:921 drmmode_output = 0x558b2dab8cc0 koutput = 0x0 props = <optimized out> drmmode = 0x558b2dab5200 mode_id = -1 i = 0 #1 0x0000558b2ba39223 in xf86DisableUnusedFunctions (pScrn=0x558b2dab4930) at ../../../../../../hw/xfree86/modes/xf86Crtc.c:3000 output = <optimized out> xf86_config = 0x558b2dab56e0 o = 6 c = <optimized out> #2 0x0000558b2ba40ef0 in xf86RandR12CrtcSet (pScreen=<optimized out>, randr_crtc=0x558b2daf51f0, randr_mode=0x0, x=0, y=0, rotation=<optimized out>, num_randr_outputs=0, randr_outputs=0x0) at ../../../../../../hw/xfree86/modes/xf86RandR12.c:1245 pScrn = 0x558b2dab4930 config = <optimized out> crtc = 0x558b2dab9ec0 transform = 0x0 changed = <optimized out> o = <optimized out> ro = <optimized out> save_crtcs = 0x558b2debcc50 save_enabled = <optimized out> #3 0x0000558b2ba84a32 in RRCrtcSet (crtc=<optimized out>, mode=0x0, x=0, y=0, rotation=rotation@entry=1, numOutputs=numOutputs@entry=0, outputs=0x0) at ../../../../randr/rrcrtc.c:768 pScreen = 0x558b2daacb80 ret = <optimized out> crtcChanged = <optimized out> o = <optimized out> #4 0x0000558b2ba862b3 in ProcRRSetCrtcConfig (client=0x558b2de43170) at ../../../../randr/rrcrtc.c:1366 stuff = <optimized out> rep = {type = 0 '\000', status = 0 '\000', sequenceNumber = 0, length = 0, newTimestamp = 766260624, pad1 = 21899, pad2 = 735925696, pad3 = 21899, pad4 = 1660946009, pad5 = 32568} pScreen = <optimized out> crtc = 0x558b2daf51f0 mode = 0x0 numOutputs = 0 outputs = <optimized out> time = <optimized out> rotation = 1 ret = <optimized out> i = <optimized out> j = <optimized out> status = <optimized out> #5 0x0000558b2b9c0e98 in Dispatch () at ../../../../dix/dispatch.c:479 result = <optimized out> start_tick = 930 #6 0x0000558b2b9c4ee0 in dix_main (argc=3, argv=0x7ffdc0d05ac8, envp=<optimized out>) at ../../../../dix/main.c:287 i = <optimized out> alwaysCheckForInput = {0, 1} #7 0x00007f3865d7db97 in __libc_start_main (main=0x558b2b9aeb50 <main>, argc=3, argv=0x7ffdc0d05ac8, init=<optimized out>, fini=<optimized out>, rtld_fini=<optimized out>, stack_end=0x7ffdc0d05ab8) at ../csu/libc-start.c:310 result = <optimized out> unwind_buf = {cancel_jmp_buf = {{jmp_buf = {0, 837132253956422676, 94056220388192, 140727838333632, 0, 0, 6878053941391146004, 6843373089882258452}, mask_was_saved = 0}}, priv = {pad = {0x0, 0x0, 0x7f3868887733 <_dl_init+259>, 0x7f3868875438}, data = {prev = 0x0, cleanup = 0x0, canceltype = 1753773875}}} not_first_call = <optimized out> #8 0x0000558b2b9aeb8a in _start ()
Unfortunately DP connectors appearing and disappearing (due to MST) is a larger project. I've been trying to get people to test my patches, but thus far without someone consistently being able to iterate. The tree is here: https://github.com/imirkin/xf86-video-nouveau At this point, I know it doesn't work, but I don't know why. (Needless to say, I don't have such a setup myself.) I believe it handles the disconnect fine, but doesn't find the new connectors on reconnect. If you could run that, and provide information (or even better, figure out why it doesn't work, since you seem like you have such capability), I'd really appreciate it.
Ilia, I tested your tree and it behaves as you expected -- it fixes the crash on undocking (I see that it adds the same koutput == NULL check) but doesn't find the external displays on docking again. I'm not sure what further information you want me to provide. I understand that you would like to finish the larger project and get external DP displays working 100%, but is there any reason why we couldn't start by committing the koutput == NULL check to fix the crash?
(In reply to John Lindgren from comment #3) > Ilia, I tested your tree and it behaves as you expected -- it fixes the > crash on undocking (I see that it adds the same koutput == NULL check) but > doesn't find the external displays on docking again. I'm not sure what > further information you want me to provide. The xorg log with my various additional prints + the output of "xrandr" after you reconnect the external displays would be a good start. > > I understand that you would like to finish the larger project and get > external DP displays working 100%, but is there any reason why we couldn't > start by committing the koutput == NULL check to fix the crash? Absolutely, I think that's a good idea. [I can't do it this second, but will try to get to it soon.]
Created attachment 140268 [details] X.org log
xrandr before undocking: Screen 0: minimum 320 x 200, current 5760 x 1200, maximum 16384 x 16384 eDP-1 connected 1920x1080+0+0 (normal left inverted right x axis y axis) 344mm x 194mm 1920x1080 60.02*+ 1680x1050 60.00 1400x1050 60.00 1280x1024 59.95 1280x960 59.99 1152x864 59.97 1024x768 59.95 800x600 59.96 640x480 59.94 720x400 59.97 640x400 59.96 640x350 59.84 DP-1 disconnected (normal left inverted right x axis y axis) DP-2 disconnected (normal left inverted right x axis y axis) DP-3 disconnected (normal left inverted right x axis y axis) DP-1-1 connected 1920x1200+1920+0 (normal left inverted right x axis y axis) 518mm x 324mm 1920x1200 59.95*+ 1920x1080 60.00 1600x1200 60.00 1680x1050 59.95 1280x1024 60.02 1280x960 60.00 1024x768 60.00 800x600 60.32 640x480 59.94 720x400 70.08 DP-1-2 connected 1920x1200+3840+0 (normal left inverted right x axis y axis) 518mm x 324mm 1920x1200 59.95*+ 1920x1080 60.00 1600x1200 60.00 1680x1050 59.95 1280x1024 60.02 1280x960 60.00 1024x768 60.00 800x600 60.32 640x480 59.94 720x400 70.08 DP-1-3 disconnected (normal left inverted right x axis y axis) xrandr after undocking: Screen 0: minimum 320 x 200, current 1920 x 1080, maximum 16384 x 16384 eDP-1 connected 1920x1080+0+0 (normal left inverted right x axis y axis) 344mm x 194mm 1920x1080 60.02*+ 1680x1050 60.00 1400x1050 60.00 1280x1024 59.95 1280x960 59.99 1152x864 59.97 1024x768 59.95 800x600 59.96 640x480 59.94 720x400 59.97 640x400 59.96 640x350 59.84 DP-1 disconnected (normal left inverted right x axis y axis) DP-2 disconnected (normal left inverted right x axis y axis) DP-3 disconnected (normal left inverted right x axis y axis) DP-1-1 disconnected (normal left inverted right x axis y axis) DP-1-2 disconnected (normal left inverted right x axis y axis) DP-1-3 disconnected (normal left inverted right x axis y axis) No difference after docking again.
Right. The MST outputs (DP-1-1, etc) never get removed, and so then they never get reinitialized. Will try to confirm with people in the know as to what the expected situation is. I probably missed something in my copying of the logic added to the modesetting driver. Thanks for that!
Ping. Can someone review this patch and if it looks good, apply it please?
I tested something very close to the patches I had, and it seemed to work fine. The monitors came back OK. Perhaps there was a kernel issue here. Not sure. I've updated my github branch, and sent an updated patch to the ML. I expect to make a release with it in the near future.
-- GitLab Migration Automatic Message -- This bug has been migrated to freedesktop.org's GitLab instance and has been closed from further activity. You can subscribe and participate further through the new bug through this link to our GitLab instance: https://gitlab.freedesktop.org/xorg/driver/xf86-video-nouveau/issues/439.
Use of freedesktop.org services, including Bugzilla, is subject to our Code of Conduct. How we collect and use information is described in our Privacy Policy.