DRM FB interface does not sanitize len when mmap'ed

Fri Jun 17 09:27:08 UTC 2022

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
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0001-drm-fb-helper-Fix-out-of-bounds-access.patch
Type: text/x-patch
Size: 3514 bytes
Desc: not available
URL: <https://lists.freedesktop.org/archives/dri-devel/attachments/20220617/29447b7d/attachment-0001.bin>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: OpenPGP_signature
Type: application/pgp-signature
Size: 840 bytes
Desc: OpenPGP digital signature
URL: <https://lists.freedesktop.org/archives/dri-devel/attachments/20220617/29447b7d/attachment-0001.sig>