Hi Am 15.06.22 um 17:00 schrieb Nuno Gonçalves:
I am crashing the kernel by doing something I believe I am allowed to do.
Using mmap to write to /dev/fb0 as the compatibility layer for Tiny
DRM vot,v220hf01a-t (ili9225).
First it happens that because of the display resolution of 220*176 the
buffer is (16 bit) 77440 bytes, which is not a multiple of the page
size (4096), unlike most regular resolution displays.
When I touch the mmap'ed virtual address above base+73728 (4096*18) up to 77439:
auto file = open("/dev/fb0", O_RDWR);
if (!file) throw;
auto fbp = (char *)mmap(0, 220 * 176 * 2, PROT_READ | PROT_WRITE,
MAP_SHARED, file, 0);
if ((size_t)fbp <= 0) throw;
fbp[220 * 176 * 2 - 2] = 0;
I get a crash:
[ 14.150463] Unable to handle kernel paging request at virtual
address ffffffc00827c000
[ 14.158640] Mem abort info:
[ 14.161626] ESR = 0x96000007
[ 14.164969] EC = 0x25: DABT (current EL), IL = 32 bits
[ 14.170470] SET = 0, FnV = 0
[ 14.173735] EA = 0, S1PTW = 0
[ 14.177047] FSC = 0x07: level 3 translation fault
[ 14.182095] Data abort info:
[ 14.185083] ISV = 0, ISS = 0x00000007
[ 14.189035] CM = 0, WnR = 0
[ 14.192107] swapper pgtable: 4k pages, 39-bit VAs, pgdp=0000000000d54000
[ 14.198997] [ffffffc00827c000] pgd=1000000001501003,
p4d=1000000001501003, pud=1000000001501003, pmd=1000000001d5c003,
pte=0000000000000000
[ 14.211992] Internal error: Oops: 96000007 [#1] PREEMPT SMP
[ 14.217659] CPU: 0 PID: 50 Comm: kworker/0:2 Not tainted 5.18.3 #18
[ 14.224027] Hardware name: Raspberry Pi Compute Module 3 Plus Rev 1.0 (DT)
[ 14.231005] Workqueue: events drm_fb_helper_damage_work
[ 14.236333] pstate: 20000005 (nzCv daif -PAN -UAO -TCO -DIT -SSBS BTYPE=--)
[ 14.243405] pc : __memcpy+0x15c/0x230
[ 14.247131] lr : drm_fb_helper_damage_work+0x25c/0x310
[ 14.252352] sp : ffffffc00822bd30
[ 14.255712] x29: ffffffc00822bd30 x28: 00000000000001b8 x27: ffffff80017e1d10
[ 14.262970] x26: ffffffc008267e80 x25: 00000000000000b0 x24: ffffff8002416800
[ 14.270228] x23: ffffff8001fd8080 x22: ffffff80017e1c00 x21: ffffff80017e1ccc
[ 14.277487] x20: ffffffc00827be80 x19: ffffff80017e1cd0 x18: ffffffe5d80bac08
[ 14.284745] x17: 0000000000000013 x16: 000000fe72080e00 x15: 0000000000000000
[ 14.292003] x14: 0000000000000000 x13: 0000000000000000 x12: 0000000000000000
[ 14.299259] x11: 0000000000000000 x10: 0000000000000000 x9 : 0000000000000000
[ 14.306517] x8 : 0000000000000000 x7 : 0000000000000000 x6 : 0000000000000000
[ 14.313772] x5 : ffffffc008268038 x4 : ffffffc00827c038 x3 : ffffffc008267fc0
[ 14.321029] x2 : 0000000000000028 x1 : ffffffc00827bfc0 x0 : ffffffc008267e80
[ 14.328288] Call trace:
[ 14.330766] __memcpy+0x15c/0x230
[ 14.334135] process_one_work+0x1dc/0x450
[ 14.338214] worker_thread+0x300/0x450
[ 14.342025] kthread+0x100/0x110
[ 14.345305] ret_from_fork+0x10/0x20
[ 14.348947] Code: a9422428 a9032c6a a9432c2a a984346c (a9c4342c)
[ 14.355132] ---[ end trace 0000000000000000 ]---
By constraining the input with this small patch it works fine:
--- a/drivers/gpu/drm/drm_fb_helper.c
+++ b/drivers/gpu/drm/drm_fb_helper.c
@@ -684,11 +684,13 @@ static void drm_fb_helper_damage(struct fb_info
*info, u32 x, u32 y,
static void drm_fb_helper_memory_range_to_clip(struct fb_info *info,
off_t off, size_t len,
struct drm_rect *clip)
{
+ struct drm_fb_helper *fb_helper = info->par;
+
off_t end = off + len;
u32 x1 = 0;
u32 y1 = off / info->fix.line_length;
u32 x2 = info->var.xres;
- u32 y2 = DIV_ROUND_UP(end, info->fix.line_length);
+ u32 y2 = min_t(u32, DIV_ROUND_UP(end, info->fix.line_length),
fb_helper->fb->height);
if ((y2 - y1) == 1) {
/*
I am sure this patch is not how it should be fixed, but I have no
knowledge of the subsystem to fix it "at the right place".
Thank you so much for reporting this bug and indicating the cause. Your fix is actually quite close. Because each scanline is rather short, the final page has a lot of empty memory. Hence, the damage handler
computes non-existing scanlines that need to be updated.Could you please try the attached patch? It's similar to your solution, but closer to the original intention of the code.
Best regards Thomas
Thanks, Nuno
-- Thomas Zimmermann Graphics Driver Developer SUSE Software Solutions Germany GmbH Maxfeldstr. 5, 90409 Nürnberg, Germany (HRB 36809, AG Nürnberg) Geschäftsführer: Ivo Totev
From e1e3bfe7bd74d5c4cfab15e05a56088d5edf82ce Mon Sep 17 00:00:00 2001
From: Thomas Zimmermann <tzimmermann@xxxxxxx>
Date: Fri, 17 Jun 2022 11:01:41 +0200
Subject: [PATCH] drm/fb-helper: Fix out-of-bounds access
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit
Clip memory range to screen-buffer size to avoid out-of-bounds access
in fbdev deferred I/O's damage handling.
Fbdev's deferred I/O can only track pages. From the range of pages, the
damage handler computes the clipping rectangle for the display update.
If the fbdev screen buffer ends near the beginning of a page, that page
could contain more scanlines. The damage handler would then track these
non-existing scanlines as dirty and provoke an out-of-bounds access
during the screen update. Hence, clip the maximum memory range to the
size of the screen buffer.
While at it, rename the variables min/max to min_off/max_off in
drm_fb_helper_deferred_io(). This avoids confusion with the macros of
the same name.
Reported-by: Nuno Gonçalves <nunojpg@xxxxxxxxx>
Signed-off-by: Thomas Zimmermann <tzimmermann@xxxxxxx>
Fixes: 67b723f5b742 ("drm/fb-helper: Calculate damaged area in separate helper")
Cc: Thomas Zimmermann <tzimmermann@xxxxxxx>
Cc: Javier Martinez Canillas <javierm@xxxxxxxxxx>
Cc: Maarten Lankhorst <maarten.lankhorst@xxxxxxxxxxxxxxx>
Cc: Maxime Ripard <mripard@xxxxxxxxxx>
Cc: <stable@xxxxxxxxxxxxxxx> # v5.18+
---
drivers/gpu/drm/drm_fb_helper.c | 27 +++++++++++++++++++--------
1 file changed, 19 insertions(+), 8 deletions(-)
diff --git a/drivers/gpu/drm/drm_fb_helper.c b/drivers/gpu/drm/drm_fb_helper.c
index 5ad2b6a2778c..81013e274935 100644
--- a/drivers/gpu/drm/drm_fb_helper.c
+++ b/drivers/gpu/drm/drm_fb_helper.c
@@ -680,7 +680,11 @@ static void drm_fb_helper_damage(struct fb_info *info, u32 x, u32 y,
schedule_work(&helper->damage_work);
}
-/* Convert memory region into area of scanlines and pixels per scanline */
+/*
+ * Convert memory region into area of scanlines and pixels per
+ * scanline. The parameters off and len must not reach beyond
+ * the framebuffer.
+ */
static void drm_fb_helper_memory_range_to_clip(struct fb_info *info, off_t off, size_t len,
struct drm_rect *clip)
{
@@ -715,22 +719,29 @@ static void drm_fb_helper_memory_range_to_clip(struct fb_info *info, off_t off,
*/
void drm_fb_helper_deferred_io(struct fb_info *info, struct list_head *pagereflist)
{
- unsigned long start, end, min, max;
+ unsigned long start, end, min_off, max_off;
struct fb_deferred_io_pageref *pageref;
struct drm_rect damage_area;
- min = ULONG_MAX;
- max = 0;
+ min_off = ULONG_MAX;
+ max_off = 0;
list_for_each_entry(pageref, pagereflist, list) {
start = pageref->offset;
end = start + PAGE_SIZE;
- min = min(min, start);
- max = max(max, end);
+ min_off = min(min_off, start);
+ max_off = max(max_off, end);
}
- if (min >= max)
+ if (min_off >= max_off)
return;
- drm_fb_helper_memory_range_to_clip(info, min, max - min, &damage_area);
+ /*
+ * As we can only track pages, we might reach beyond the end
+ * of the screen and account for non-existing scanlines. Hence,
+ * keep the covered memory area within the screen buffer.
+ */
+ max_off = min(max_off, info->screen_size);
+
+ drm_fb_helper_memory_range_to_clip(info, min_off, max_off - min_off, &damage_area);
drm_fb_helper_damage(info, damage_area.x1, damage_area.y1,
drm_rect_width(&damage_area),
drm_rect_height(&damage_area));
--
2.36.1
Attachment:
OpenPGP_signature
Description: OpenPGP digital signature
