Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:
Pre-existing issues: - [Critical] The driver passes a virtual address to __free_pages(), causing an out-of-bounds pointer dereference and kernel panic. - [High] Ignored return value of fb_deferred_io_init() leads to NULL pointer dereferences in mmap, open, and cleanup error paths. - [High] Missing .driver_data initialization in i2c_device_id array leads to NULL pointer dereference on sysfs instantiation. -- commit d46565518c57c096a66d45b42bbe529fae5740d7 Author: Lorenzo Stoakes (ARM) <[email protected]> fbdev: defio: assert FBINFO_VIRTFB, drop VM_IO, add VM_MIXEDMAP Currently all drivers which use defio allocate system memory. All of them also set FBINFO_VIRTFB, other than ssd1307fb, however this driver allocates system RAM, so simply failed to set this flag when it ought to. > diff --git a/drivers/video/fbdev/ssd1307fb.c b/drivers/video/fbdev/ssd1307fb.c > index c4fdecafd8560..958514a354338 100644 > --- a/drivers/video/fbdev/ssd1307fb.c > +++ b/drivers/video/fbdev/ssd1307fb.c > @@ -763,6 +763,8 @@ static int ssd1307fb_probe(struct i2c_client *client) > info->fix.smem_start = __pa(vmem); > info->fix.smem_len = vmem_size; > > + info->flags = FBINFO_VIRTFB; > + > fb_deferred_io_init(info); [Severity: High] This isn't a bug introduced by this patch, but if memory allocation fails in fb_deferred_io_init(), it returns -ENOMEM but ssd1307fb_probe() ignores it, leaving info->fbdefio_state as NULL and info->deferred_work uninitialized. Subsequent probe failure or device removal calls fb_deferred_io_cleanup(), which can unconditionally call flush_delayed_work() on the uninitialized workqueue and then dereference the NULL fbdefio_state: drivers/video/fbdev/core/fb_defio.c:fb_deferred_io_cleanup() { ... mutex_lock(&fbdefio_state->lock); ... } Additionally, userspace calling open() or mmap() could trigger NULL pointer dereferences in fb_deferred_io_open() and fb_deferred_io_mmap(). Does the ignored return value of fb_deferred_io_init() lead to NULL pointer dereferences in mmap, open, and cleanup error paths? > > i2c_set_clientdata(client, info); [Severity: Critical] This is a pre-existing issue, but the driver allocates video memory using __get_free_pages(), which returns a virtual address (void *). During probe failure, it passes this virtual address directly to __free_pages(): drivers/video/fbdev/ssd1307fb.c:ssd1307fb_probe() { ... fb_defio_error: __free_pages(vmem, get_order(vmem_size)); ... } And during module unload, it does the same: drivers/video/fbdev/ssd1307fb.c:ssd1307fb_remove() { ... __free_pages(__va(info->fix.smem_start), get_order(info->fix.smem_len)); ... } Because C implicitly casts void * to struct page *, the kernel could treat the zeroed video memory as a page struct, reading garbage PFNs, corrupting the buddy allocator state, and crashing. Does passing a virtual address to __free_pages() in these paths cause an out-of-bounds pointer dereference and kernel panic? Should this be using free_pages() instead? [Severity: High] This isn't a bug introduced by this patch, but if the driver is instantiated via the legacy I2C sysfs interface, device_get_match_data() falls back to matching against ssd1307fb_i2c_id: drivers/video/fbdev/ssd1307fb.c:ssd1307fb_i2c_id[] = { { .name = "ssd1305fb" }, ... }; Because entries in this array omit the .driver_data field, it defaults to NULL. The probe function then unconditionally dereferences this NULL pointer: drivers/video/fbdev/ssd1307fb.c:ssd1307fb_probe() { ... par->vcomh = par->device_info->default_vcomh; ... } Does the missing .driver_data initialization in the i2c_device_id array lead to a NULL pointer dereference on sysfs instantiation? -- Sashiko AI review ยท https://sashiko.dev/#/patchset/20260914-b4-mmap-prepare-vma-flag-sanify-v2-0-7d9781ed5...@kernel.org?part=19
