On Mon, 18 Dec 2023, Sasha Levin <sashal@xxxxxxxxxx> wrote: > From: Ziqi Zhao <astrajoan@xxxxxxxxx> > > [ Upstream commit 3823119b9c2b5f9e9b760336f75bc989b805cde6 ] > > The connector_set contains uninitialized values when allocated with > kmalloc_array. However, in the "out" branch, the logic assumes that any > element in connector_set would be equal to NULL if failed to > initialize, which causes the bug reported by Syzbot. The fix is to use > an extra variable to keep track of how many connectors are initialized > indeed, and use that variable to decrease any refcounts in the "out" > branch. > > Reported-by: syzbot+4fad2e57beb6397ab2fc@xxxxxxxxxxxxxxxxxxxxxxxxx > Signed-off-by: Ziqi Zhao <astrajoan@xxxxxxxxx> > Reported-and-tested-by: syzbot+4fad2e57beb6397ab2fc@xxxxxxxxxxxxxxxxxxxxxxxxx > Tested-by: Harshit Mogalapalli <harshit.m.mogalapalli@xxxxxxxxxx> > Link: https://lore.kernel.org/r/20230721161446.8602-1-astrajoan@xxxxxxxxx > Signed-off-by: Maxime Ripard <mripard@xxxxxxxxxx> > Signed-off-by: Sasha Levin <sashal@xxxxxxxxxx> This commit fixes an uninitialized value, but introduces a new one. Please backport 6e455f5dcdd1 ("drm/crtc: fix uninitialized variable use") from v6.7-rc6 to go with it. Thanks, Jani. > --- > drivers/gpu/drm/drm_crtc.c | 9 +++++---- > 1 file changed, 5 insertions(+), 4 deletions(-) > > diff --git a/drivers/gpu/drm/drm_crtc.c b/drivers/gpu/drm/drm_crtc.c > index 5af25ce5bf7c2..5ae3adfbc5e80 100644 > --- a/drivers/gpu/drm/drm_crtc.c > +++ b/drivers/gpu/drm/drm_crtc.c > @@ -556,8 +556,7 @@ int drm_mode_setcrtc(struct drm_device *dev, void *data, > struct drm_mode_set set; > uint32_t __user *set_connectors_ptr; > struct drm_modeset_acquire_ctx ctx; > - int ret; > - int i; > + int ret, i, num_connectors; > > if (!drm_core_check_feature(dev, DRIVER_MODESET)) > return -EINVAL; > @@ -672,6 +671,7 @@ int drm_mode_setcrtc(struct drm_device *dev, void *data, > goto out; > } > > + num_connectors = 0; > for (i = 0; i < crtc_req->count_connectors; i++) { > connector_set[i] = NULL; > set_connectors_ptr = (uint32_t __user *)(unsigned long)crtc_req->set_connectors_ptr; > @@ -692,6 +692,7 @@ int drm_mode_setcrtc(struct drm_device *dev, void *data, > connector->name); > > connector_set[i] = connector; > + num_connectors++; > } > } > > @@ -700,7 +701,7 @@ int drm_mode_setcrtc(struct drm_device *dev, void *data, > set.y = crtc_req->y; > set.mode = mode; > set.connectors = connector_set; > - set.num_connectors = crtc_req->count_connectors; > + set.num_connectors = num_connectors; > set.fb = fb; > ret = __drm_mode_set_config_internal(&set, &ctx); > > @@ -709,7 +710,7 @@ int drm_mode_setcrtc(struct drm_device *dev, void *data, > drm_framebuffer_put(fb); > > if (connector_set) { > - for (i = 0; i < crtc_req->count_connectors; i++) { > + for (i = 0; i < num_connectors; i++) { > if (connector_set[i]) > drm_connector_put(connector_set[i]); > } -- Jani Nikula, Intel
