From: Daniel Vetter <daniel@ffwll.ch>
To: Thomas Zimmermann <tzimmermann@suse.de>
Cc: daniel@ffwll.ch, deller@gmx.de, sudipm.mukherjee@gmail.com,
sam@ravnborg.org, javierm@redhat.com, zackr@vmware.com,
hdegoede@redhat.com, linux-fbdev@vger.kernel.org,
dri-devel@lists.freedesktop.org, stable@vger.kernel.org,
Daniel Vetter <daniel.vetter@ffwll.ch>,
Zheyu Ma <zheyuma97@gmail.com>,
Xiyu Yang <xiyuyang19@fudan.edu.cn>,
Zhen Lei <thunder.leizhen@huawei.com>,
Matthew Wilcox <willy@infradead.org>,
Alex Deucher <alexander.deucher@amd.com>,
Tetsuo Handa <penguin-kernel@i-love.sakura.ne.jp>,
Guenter Roeck <linux@roeck-us.net>
Subject: Re: [PATCH] fbdev: Fix unregistering of framebuffers without device
Date: Tue, 5 Apr 2022 11:01:50 +0200 [thread overview]
Message-ID: <YkwFfusqI2Nuu7Dn@phenom.ffwll.local> (raw)
In-Reply-To: <20220404194402.29974-1-tzimmermann@suse.de>
On Mon, Apr 04, 2022 at 09:44:02PM +0200, Thomas Zimmermann wrote:
> OF framebuffers do not have an underlying device in the Linux
> device hierarchy. Do a regular unregister call instead of hot
> unplugging such a non-existing device. Fixes a NULL dereference.
> An example error message on ppc64le is shown below.
>
> BUG: Kernel NULL pointer dereference on read at 0x00000060
> Faulting instruction address: 0xc00000000080dfa4
> Oops: Kernel access of bad area, sig: 11 [#1]
> LE PAGE_SIZE=64K MMU=Hash SMP NR_CPUS=2048 NUMA pSeries
> [...]
> CPU: 2 PID: 139 Comm: systemd-udevd Not tainted 5.17.0-ae085d7f9365 #1
> NIP: c00000000080dfa4 LR: c00000000080df9c CTR: c000000000797430
> REGS: c000000004132fe0 TRAP: 0300 Not tainted (5.17.0-ae085d7f9365)
> MSR: 8000000002009033 <SF,VEC,EE,ME,IR,DR,RI,LE> CR: 28228282 XER: 20000000
> CFAR: c00000000000c80c DAR: 0000000000000060 DSISR: 40000000 IRQMASK: 0
> GPR00: c00000000080df9c c000000004133280 c00000000169d200 0000000000000029
> GPR04: 00000000ffffefff c000000004132f90 c000000004132f88 0000000000000000
> GPR08: c0000000015658f8 c0000000015cd200 c0000000014f57d0 0000000048228283
> GPR12: 0000000000000000 c00000003fffe300 0000000020000000 0000000000000000
> GPR16: 0000000000000000 0000000113fc4a40 0000000000000005 0000000113fcfb80
> GPR20: 000001000f7283b0 0000000000000000 c000000000e4a588 c000000000e4a5b0
> GPR24: 0000000000000001 00000000000a0000 c008000000db0168 c0000000021f6ec0
> GPR28: c0000000016d65a8 c000000004b36460 0000000000000000 c0000000016d64b0
> NIP [c00000000080dfa4] do_remove_conflicting_framebuffers+0x184/0x1d0
> [c000000004133280] [c00000000080df9c] do_remove_conflicting_framebuffers+0x17c/0x1d0 (unreliable)
> [c000000004133350] [c00000000080e4d0] remove_conflicting_framebuffers+0x60/0x150
> [c0000000041333a0] [c00000000080e6f4] remove_conflicting_pci_framebuffers+0x134/0x1b0
> [c000000004133450] [c008000000e70438] drm_aperture_remove_conflicting_pci_framebuffers+0x90/0x100 [drm]
> [c000000004133490] [c008000000da0ce4] bochs_pci_probe+0x6c/0xa64 [bochs]
> [...]
> [c000000004133db0] [c00000000002aaa0] system_call_exception+0x170/0x2d0
> [c000000004133e10] [c00000000000c3cc] system_call_common+0xec/0x250
>
> The bug [1] was introduced by commit 27599aacbaef ("fbdev: Hot-unplug
> firmware fb devices on forced removal"). Most firmware framebuffers
> have an underlying platform device, which can be hot-unplugged
> before loading the native graphics driver. OF framebuffers do not
> (yet) have that device. Fix the code by unregistering the framebuffer
> as before without a hot unplug.
>
> Tested with 5.17 on qemu ppc64le emulation.
>
> Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
> Fixes: 27599aacbaef ("fbdev: Hot-unplug firmware fb devices on forced removal")
> Reported-by: Sudip Mukherjee <sudipm.mukherjee@gmail.com>
> Cc: Zack Rusin <zackr@vmware.com>
> Cc: Javier Martinez Canillas <javierm@redhat.com>
> Cc: Hans de Goede <hdegoede@redhat.com>
> Cc: stable@vger.kernel.org # v5.11+
> Cc: Helge Deller <deller@gmx.de>
> Cc: Daniel Vetter <daniel.vetter@ffwll.ch>
> Cc: Sam Ravnborg <sam@ravnborg.org>
> Cc: Zheyu Ma <zheyuma97@gmail.com>
> Cc: Xiyu Yang <xiyuyang19@fudan.edu.cn>
> Cc: Zhen Lei <thunder.leizhen@huawei.com>
> Cc: Matthew Wilcox <willy@infradead.org>
> Cc: Alex Deucher <alexander.deucher@amd.com>
> Cc: Tetsuo Handa <penguin-kernel@i-love.sakura.ne.jp>
> Cc: Guenter Roeck <linux@roeck-us.net>
> Cc: linux-fbdev@vger.kernel.org
> Cc: dri-devel@lists.freedesktop.org
> Link: https://lore.kernel.org/all/YkHXO6LGHAN0p1pq@debian/ # [1]
> ---
> drivers/video/fbdev/core/fbmem.c | 9 ++++++++-
> 1 file changed, 8 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/video/fbdev/core/fbmem.c b/drivers/video/fbdev/core/fbmem.c
> index 34d6bb1bf82e..a6bb0e438216 100644
> --- a/drivers/video/fbdev/core/fbmem.c
> +++ b/drivers/video/fbdev/core/fbmem.c
> @@ -1579,7 +1579,14 @@ static void do_remove_conflicting_framebuffers(struct apertures_struct *a,
> * If it's not a platform device, at least print a warning. A
> * fix would add code to remove the device from the system.
> */
> - if (dev_is_platform(device)) {
> + if (!device) {
> + /* TODO: Represent each OF framebuffer as its own
> + * device in the device hierarchy. For now, offb
> + * doesn't have such a device, so unregister the
> + * framebuffer as before without warning.
> + */
> + do_unregister_framebuffer(registered_fb[i]);
Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch>
Might be good to have a fb_info flag for offb and then check in
register_framebuffer that everyone else does have a device? Just to make
sure we don't have more surprises here ...
-Daniel
> + } else if (dev_is_platform(device)) {
> registered_fb[i]->forced_out = true;
> platform_device_unregister(to_platform_device(device));
> } else {
> --
> 2.35.1
>
--
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
next prev parent reply other threads:[~2022-04-05 10:04 UTC|newest]
Thread overview: 5+ messages / expand[flat|nested] mbox.gz Atom feed top
2022-04-04 19:44 [PATCH] fbdev: Fix unregistering of framebuffers without device Thomas Zimmermann
2022-04-05 9:01 ` Daniel Vetter [this message]
2022-04-05 18:24 ` Thomas Zimmermann
2022-04-05 13:05 ` Sudip Mukherjee
2022-04-05 14:08 ` [PATCH] " Javier Martinez Canillas
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=YkwFfusqI2Nuu7Dn@phenom.ffwll.local \
--to=daniel@ffwll.ch \
--cc=alexander.deucher@amd.com \
--cc=daniel.vetter@ffwll.ch \
--cc=deller@gmx.de \
--cc=dri-devel@lists.freedesktop.org \
--cc=hdegoede@redhat.com \
--cc=javierm@redhat.com \
--cc=linux-fbdev@vger.kernel.org \
--cc=linux@roeck-us.net \
--cc=penguin-kernel@i-love.sakura.ne.jp \
--cc=sam@ravnborg.org \
--cc=stable@vger.kernel.org \
--cc=sudipm.mukherjee@gmail.com \
--cc=thunder.leizhen@huawei.com \
--cc=tzimmermann@suse.de \
--cc=willy@infradead.org \
--cc=xiyuyang19@fudan.edu.cn \
--cc=zackr@vmware.com \
--cc=zheyuma97@gmail.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).