Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
Pre-existing issues: - [High] A malicious or misconfigured USB device can cause a NULL pointer dereference and kernel panic in `drm_object_attach_property` by returning `GUD_PROPERTY_TV_MODE` after any other non-margin TV property (such as `GUD_PROPERTY_TV_BRIGHTNESS` or `GUD_PROPERTY_TV_HUE`). -- --- Patch [1]: [PATCH] drm/gud: NUL-terminate TV mode names read from the device --- commit 789868bcbf6cc6ddf0d3b74feab3a22914863ec8 Author: Deepanshu Kartikey <[email protected]> drm/gud: NUL-terminate TV mode names read from the device This commit fixes a potential out-of-bounds read by explicitly NUL-terminating TV mode names read from the USB device. This ensures strlen() won't run past the end of the allocation when passed to drm_mode_create_tv_properties_legacy(). Fixes: 40e1a70b4aed ("drm: Add GUD USB Display driver") Reported-by: [email protected] Closes: https://syzkaller.appspot.com/bug?extid=916c888ba5f1a54c9526 Tested-by: [email protected] Signed-off-by: Deepanshu Kartikey <[email protected]> > diff --git a/drivers/gpu/drm/gud/gud_connector.c > b/drivers/gpu/drm/gud/gud_connector.c > index ea0cca58b7c89..5c0065c876a77 100644 > --- a/drivers/gpu/drm/gud/gud_connector.c > +++ b/drivers/gpu/drm/gud/gud_connector.c > @@ -396,8 +396,13 @@ static int gud_connector_add_tv_mode(struct gud_device > *gdrm, struct drm_connect > } > > num_modes = ret / GUD_CONNECTOR_TV_MODE_NAME_LEN; > - for (i = 0; i < num_modes; i++) > - modes[i] = &buf[i * GUD_CONNECTOR_TV_MODE_NAME_LEN]; > + for (i = 0; i < num_modes; i++) { > + char *mode = &buf[i * GUD_CONNECTOR_TV_MODE_NAME_LEN]; > + > + /* The device is not trusted to NUL-terminate the name */ > + mode[GUD_CONNECTOR_TV_MODE_NAME_LEN - 1] = '\0'; > + modes[i] = mode; > + } > > ret = drm_mode_create_tv_properties_legacy(connector->dev, num_modes, > modes); [Severity: High] This is a pre-existing issue, but could a malicious or misconfigured USB device cause a NULL pointer dereference and kernel panic by returning GUD_PROPERTY_TV_MODE after another non-margin TV property? In gud_connector_add_properties(), properties from the USB device are processed sequentially. If a property like GUD_PROPERTY_TV_BRIGHTNESS is processed first, drm_mode_create_tv_properties_legacy() creates the baseline TV properties but leaves legacy_tv_mode_property as NULL since num_modes is 0. If GUD_PROPERTY_TV_MODE is processed next, it calls gud_connector_add_tv_mode() which then calls drm_mode_create_tv_properties_legacy() here: drivers/gpu/drm/drm_connector.c:drm_mode_create_tv_properties_legacy() { ... if (dev->mode_config.tv_select_subconnector_property) return 0; ... } This early return prevents dev->mode_config.legacy_tv_mode_property from being created. Then in gud_connector_add_properties(), gud_connector_property_lookup() will return NULL for legacy_tv_mode_property: drivers/gpu/drm/gud/gud_connector.c:gud_connector_add_properties() { ... property = gud_connector_property_lookup(connector, prop); if (drm_WARN_ON(drm, IS_ERR(property))) continue; ... } Since IS_ERR() ignores NULL, the NULL property pointer bypasses this check and is passed to drm_object_attach_property(), which unconditionally dereferences it: drivers/gpu/drm/drm_mode_object.c:drm_object_attach_property() { ... struct drm_device *dev = property->dev; ... } Could this lead to a local denial of service? > free: > kfree(buf); > if (ret < 0) -- Sashiko AI review ยท https://sashiko.dev/#/patchset/[email protected]?part=1
