vmw_fb_kms_framebuffer() declares a drm_mode_fb_cmd2 struct on the stack without zero-ing it and then continues with initializing only some fields.
This leads to drm_mode_fb_cmd2.modifiers[0] containing garbage, which eventually gets used by drm_helper_mode_fill_fb_struct() to set fb->modifier when leads to the following atomic-check failure:
vmwgfx 0000:00:02.0: [drm:drm_atomic_check_only] [PLANE:34:plane-0] invalid pixel format XR24 little-endian (0x34325258), modifier 0xffff94d64719e000 fbcon_init: detected unhandled fb_set_par error, error code -22
Which causes the fbdev emulation and thus also fbcon to not work.
Initialize the struct with all zeros to fix this.
Fixes: dabdcdc9822a ("drm/vmwgfx: Switch to mode_cmd2") BugLink: https://bugzilla.redhat.com/show_bug.cgi?id=2072556 Signed-off-by: Hans de Goede hdegoede@redhat.com --- While working on this I noticed that at least the vmwgfx_fb.c code has a tendency to declare structs on the stack without zero-ing them, instead relying on manually initializing all the fields. Specifically struct drm_mode_set is used multiple times like this and there also is an occurence of struct drm_clip being used like this, but that one is less worrying.
This is a bad idea because some of these structs may get new fields added later. Someone really should do an audit of the entire vmwgfx code for this pattern and make sure all structs declared on the stack are zero-ed before use. --- drivers/gpu/drm/vmwgfx/vmwgfx_fb.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_fb.c b/drivers/gpu/drm/vmwgfx/vmwgfx_fb.c index 8ee34576c7d0..7509013818ba 100644 --- a/drivers/gpu/drm/vmwgfx/vmwgfx_fb.c +++ b/drivers/gpu/drm/vmwgfx/vmwgfx_fb.c @@ -483,7 +483,7 @@ static int vmw_fb_kms_detach(struct vmw_fb_par *par,
static int vmw_fb_kms_framebuffer(struct fb_info *info) { - struct drm_mode_fb_cmd2 mode_cmd; + struct drm_mode_fb_cmd2 mode_cmd = {}; struct vmw_fb_par *par = info->par; struct fb_var_screeninfo *var = &info->var; struct drm_framebuffer *cur_fb;
Hello Hans,
On 5/9/22 13:04, Hans de Goede wrote:
vmw_fb_kms_framebuffer() declares a drm_mode_fb_cmd2 struct on the stack without zero-ing it and then continues with initializing only some fields.
This leads to drm_mode_fb_cmd2.modifiers[0] containing garbage, which eventually gets used by drm_helper_mode_fill_fb_struct() to set fb->modifier when leads to the following atomic-check failure:
vmwgfx 0000:00:02.0: [drm:drm_atomic_check_only] [PLANE:34:plane-0] invalid pixel format XR24 little-endian (0x34325258), modifier 0xffff94d64719e000 fbcon_init: detected unhandled fb_set_par error, error code -22
Which causes the fbdev emulation and thus also fbcon to not work.
Initialize the struct with all zeros to fix this.
Fixes: dabdcdc9822a ("drm/vmwgfx: Switch to mode_cmd2") BugLink: https://bugzilla.redhat.com/show_bug.cgi?id=2072556 Signed-off-by: Hans de Goede hdegoede@redhat.com
Zack fixed this already:
https://cgit.freedesktop.org/drm/drm-misc/commit/?id=5405d25b9e8e6
Hi,
On 5/9/22 13:52, Javier Martinez Canillas wrote:
Hello Hans,
On 5/9/22 13:04, Hans de Goede wrote:
vmw_fb_kms_framebuffer() declares a drm_mode_fb_cmd2 struct on the stack without zero-ing it and then continues with initializing only some fields.
This leads to drm_mode_fb_cmd2.modifiers[0] containing garbage, which eventually gets used by drm_helper_mode_fill_fb_struct() to set fb->modifier when leads to the following atomic-check failure:
vmwgfx 0000:00:02.0: [drm:drm_atomic_check_only] [PLANE:34:plane-0] invalid pixel format XR24 little-endian (0x34325258), modifier 0xffff94d64719e000 fbcon_init: detected unhandled fb_set_par error, error code -22
Which causes the fbdev emulation and thus also fbcon to not work.
Initialize the struct with all zeros to fix this.
Fixes: dabdcdc9822a ("drm/vmwgfx: Switch to mode_cmd2") BugLink: https://bugzilla.redhat.com/show_bug.cgi?id=2072556 Signed-off-by: Hans de Goede hdegoede@redhat.com
Zack fixed this already:
https://cgit.freedesktop.org/drm/drm-misc/commit/?id=5405d25b9e8e6
I see, but it seems that this was never pushed to drm-misc-fixes, so this is still broken in 5.18-rc#
Regards,
Hans
On 5/9/22 13:55, Hans de Goede wrote:
[snip]
Fixes: dabdcdc9822a ("drm/vmwgfx: Switch to mode_cmd2") BugLink: https://bugzilla.redhat.com/show_bug.cgi?id=2072556 Signed-off-by: Hans de Goede hdegoede@redhat.com
Zack fixed this already:
https://cgit.freedesktop.org/drm/drm-misc/commit/?id=5405d25b9e8e6
I see, but it seems that this was never pushed to drm-misc-fixes, so this is still broken in 5.18-rc#
Indeed. Agreed that should be cherry-picked in -fixes as well.
CCing airlied
On 09.05.22 14:02, Javier Martinez Canillas wrote:
On 5/9/22 13:55, Hans de Goede wrote: [snip]
Fixes: dabdcdc9822a ("drm/vmwgfx: Switch to mode_cmd2") BugLink: https://bugzilla.redhat.com/show_bug.cgi?id=2072556 Signed-off-by: Hans de Goede hdegoede@redhat.com
Zack fixed this already:
https://cgit.freedesktop.org/drm/drm-misc/commit/?id=5405d25b9e8e6
I see, but it seems that this was never pushed to drm-misc-fixes, so this is still broken in 5.18-rc#
Indeed. Agreed that should be cherry-picked in -fixes as well.
Looks to me like nobody did that and this regression fix is missing in the pull request Dave sent to Linus earlier today. Can anybody please make sure it makes the next weeks pull? Or was there a change of plans?
Ciao, Thorsten (wearing his 'the Linux kernel's regression tracker' hat)
P.S.: As the Linux kernel's regression tracker I deal with a lot of reports and sometimes miss something important when writing mails like this. If that's the case here, don't hesitate to tell me in a public reply, it's in everyone's interest to set the public record straight.
On May 13, 2022, at 3:43 AM, Thorsten Leemhuis <regressions@leemhuis.infomailto:regressions@leemhuis.info> wrote:
CCing airlied
On 09.05.22 14:02, Javier Martinez Canillas wrote: On 5/9/22 13:55, Hans de Goede wrote: [snip]
Fixes: dabdcdc9822a ("drm/vmwgfx: Switch to mode_cmd2") BugLink: https://nam04.safelinks.protection.outlook.com/?url=https%3A%2F%2Fbugzilla.r... Signed-off-by: Hans de Goede <hdegoede@redhat.commailto:hdegoede@redhat.com> ---
Zack fixed this already:
https://nam04.safelinks.protection.outlook.com/?url=https%3A%2F%2Fcgit.freed...
I see, but it seems that this was never pushed to drm-misc-fixes, so this is still broken in 5.18-rc#
Indeed. Agreed that should be cherry-picked in -fixes as well.
Looks to me like nobody did that and this regression fix is missing in
No, it has been done. It’s after rc6 so the patch is in drm-misc-next-fixes you can see it at: https://cgit.freedesktop.org/drm/drm-misc/commit/drivers/gpu/drm/vmwgfx?h=dr...
z
On 13.05.22 15:25, Zack Rusin wrote:
On May 13, 2022, at 3:43 AM, Thorsten Leemhuis <regressions@leemhuis.info mailto:regressions@leemhuis.info> wrote:
CCing airlied
On 09.05.22 14:02, Javier Martinez Canillas wrote:
On 5/9/22 13:55, Hans de Goede wrote: [snip]
Fixes: dabdcdc9822a ("drm/vmwgfx: Switch to mode_cmd2") BugLink: https://nam04.safelinks.protection.outlook.com/?url=https%3A%2F%2Fbugzilla.r... https://nam04.safelinks.protection.outlook.com/?url=https%3A%2F%2Fbugzilla.redhat.com%2Fshow_bug.cgi%3Fid%3D2072556&data=05%7C01%7Czackr%40vmware.com%7Ca34647a1351748917ad608da34b459a0%7Cb39138ca3cee4b4aa4d6cd83d9dd62f0%7C0%7C0%7C637880246471436744%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000%7C%7C%7C&sdata=LSQP%2Bqnf4p51QsF%2B7ZkvKlB5gSx0%2FgRUsgcIPChR33g%3D&reserved=0 Signed-off-by: Hans de Goede <hdegoede@redhat.com
mailto:hdegoede@redhat.com>
Zack fixed this already:
https://nam04.safelinks.protection.outlook.com/?url=https%3A%2F%2Fcgit.freed... https://nam04.safelinks.protection.outlook.com/?url=https%3A%2F%2Fcgit.freedesktop.org%2Fdrm%2Fdrm-misc%2Fcommit%2F%3Fid%3D5405d25b9e8e6&data=05%7C01%7Czackr%40vmware.com%7Ca34647a1351748917ad608da34b459a0%7Cb39138ca3cee4b4aa4d6cd83d9dd62f0%7C0%7C0%7C637880246471436744%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000%7C%7C%7C&sdata=IMtLj94VBo3Bv8oCGmbatBmT%2F2%2B9xkIptnlPTPuHMHc%3D&reserved=0
I see, but it seems that this was never pushed to drm-misc-fixes, so this is still broken in 5.18-rc#
Indeed. Agreed that should be cherry-picked in -fixes as well.
Looks to me like nobody did that and this regression fix is missing in
No, it has been done. It’s after rc6 so the patch is in drm-misc-next-fixes you can see it at: https://cgit.freedesktop.org/drm/drm-misc/commit/drivers/gpu/drm/vmwgfx?h=dr... https://cgit.freedesktop.org/drm/drm-misc/commit/drivers/gpu/drm/vmwgfx?h=drm-misc-next-fixes&id=5405d25b9e8e6e0d3bdb04833d528a9bb35fe7ce
Sorry, I try, but keeping track of subsystem specific rules and branches is hard -- and Javier actually asked for "drm-misc-fixes".
That being said: I'd like to question the approach here, as the docs for handling regression clearly explain that this regression fix should be applied this cycle: https://www.kernel.org/doc/html/latest/process/handling-regressions.html
[background: the fixes tag in the change mentions an older commit, but it sees the problem surfaced post-5.17 cycle, as Hand pointed out at the start of this thread where he linked here: https://bugzilla.redhat.com/show_bug.cgi?id=2072556 ]
Sure, there are fixes that are dangerous and maybe should wait for the next cycle, but from what I see this one doesn't look like one of those. And if we don't fix this now many more people will run into this.
Dave, Daniel, could you please consider picking this up?
Ciao, Thorsten (wearing his 'the Linux kernel's regression tracker' hat)
P.S.: As the Linux kernel's regression tracker I deal with a lot of reports and sometimes miss something important when writing mails like this. If that's the case here, don't hesitate to tell me in a public reply, it's in everyone's interest to set the public record straight.
Hi,
On 5/13/22 15:25, Zack Rusin wrote:
On May 13, 2022, at 3:43 AM, Thorsten Leemhuis <regressions@leemhuis.info mailto:regressions@leemhuis.info> wrote:
CCing airlied
On 09.05.22 14:02, Javier Martinez Canillas wrote:
On 5/9/22 13:55, Hans de Goede wrote: [snip]
Fixes: dabdcdc9822a ("drm/vmwgfx: Switch to mode_cmd2") BugLink: https://nam04.safelinks.protection.outlook.com/?url=https%3A%2F%2Fbugzilla.r... https://nam04.safelinks.protection.outlook.com/?url=https%3A%2F%2Fbugzilla.redhat.com%2Fshow_bug.cgi%3Fid%3D2072556&data=05%7C01%7Czackr%40vmware.com%7Ca34647a1351748917ad608da34b459a0%7Cb39138ca3cee4b4aa4d6cd83d9dd62f0%7C0%7C0%7C637880246471436744%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000%7C%7C%7C&sdata=LSQP%2Bqnf4p51QsF%2B7ZkvKlB5gSx0%2FgRUsgcIPChR33g%3D&reserved=0 Signed-off-by: Hans de Goede <hdegoede@redhat.com mailto:hdegoede@redhat.com>
Zack fixed this already:
https://nam04.safelinks.protection.outlook.com/?url=https%3A%2F%2Fcgit.freed... https://nam04.safelinks.protection.outlook.com/?url=https%3A%2F%2Fcgit.freedesktop.org%2Fdrm%2Fdrm-misc%2Fcommit%2F%3Fid%3D5405d25b9e8e6&data=05%7C01%7Czackr%40vmware.com%7Ca34647a1351748917ad608da34b459a0%7Cb39138ca3cee4b4aa4d6cd83d9dd62f0%7C0%7C0%7C637880246471436744%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000%7C%7C%7C&sdata=IMtLj94VBo3Bv8oCGmbatBmT%2F2%2B9xkIptnlPTPuHMHc%3D&reserved=0
I see, but it seems that this was never pushed to drm-misc-fixes, so this is still broken in 5.18-rc#
Indeed. Agreed that should be cherry-picked in -fixes as well.
Looks to me like nobody did that and this regression fix is missing in
No, it has been done. It’s after rc6 so the patch is in drm-misc-next-fixes you can see it at: https://cgit.freedesktop.org/drm/drm-misc/commit/drivers/gpu/drm/vmwgfx?h=dr... https://cgit.freedesktop.org/drm/drm-misc/commit/drivers/gpu/drm/vmwgfx?h=drm-misc-next-fixes&id=5405d25b9e8e6e0d3bdb04833d528a9bb35fe7ce
Erm, the after rc6 rule is for fixes for the next cycle, if we want to get this in this cycle (which IMHO we want) it should have been in drm-misc-fixes before the PR which was send to Linus today.
At this point in the cycle the best thing is probably to just send it directly to Linus. That or wait till 5.19, but it seems that for some reason (likely changes elsewhere changing the stack) this long standing bugs is hit 100% with 5.18, where as before we seemed to be lucky and not hit it.
Regards,
Hans
On Fri, 2022-05-13 at 16:18 +0200, Hans de Goede wrote:
Hi,
On 5/13/22 15:25, Zack Rusin wrote:
On May 13, 2022, at 3:43 AM, Thorsten Leemhuis <regressions@leemhuis.info mailto:regressions@leemhuis.info> wrote:
CCing airlied
On 09.05.22 14:02, Javier Martinez Canillas wrote:
On 5/9/22 13:55, Hans de Goede wrote: [snip]
> > Fixes: dabdcdc9822a ("drm/vmwgfx: Switch to mode_cmd2") > BugLink: > https://nam04.safelinks.protection.outlook.com/?url=https%3A%2F%2Fbugzilla.r... > < > https://nam04.safelinks.protection.outlook.com/?url=https%3A%2F%2Fbugzilla.r... > > > Signed-off-by: Hans de Goede > <hdegoede@redhat.com mailto:hdegoede@redhat.com> > ---
Zack fixed this already:
https://nam04.safelinks.protection.outlook.com/?url=https%3A%2F%2Fcgit.freed... < https://nam04.safelinks.protection.outlook.com/?url=https%3A%2F%2Fcgit.freed... >
I see, but it seems that this was never pushed to drm-misc-fixes, so this is still broken in 5.18-rc#
Indeed. Agreed that should be cherry-picked in -fixes as well.
Looks to me like nobody did that and this regression fix is missing in
No, it has been done. It’s after rc6 so the patch is in drm-misc-next-fixes you can see it at: https://nam04.safelinks.protection.outlook.com/?url=https%3A%2F%2Fcgit.freed... < https://nam04.safelinks.protection.outlook.com/?url=https%3A%2F%2Fcgit.freed...
Erm, the after rc6 rule is for fixes for the next cycle, if we want to get this in this cycle (which IMHO we want) it should have been in drm-misc-fixes before the PR which was send to Linus today.
At this point in the cycle the best thing is probably to just send it directly to Linus. That or wait till 5.19, but it seems that for some reason (likely changes elsewhere changing the stack) this long standing bugs is hit 100% with 5.18, where as before we seemed to be lucky and not hit it.
Hmm, yes, agreed. Just in case Dave was planning to queue up something else I've sent a pull request to Dave and Daniel for those changes. If they'll say we should send it to Linus directly then I'll do that.
z
dri-devel@lists.freedesktop.org
-
Hans de Goede -
Javier Martinez Canillas -
Thorsten Leemhuis -
Zack Rusin