All of lore.kernel.org
 help / color / mirror / Atom feed
From: sashiko-bot@kernel.org
To: "Maxime Ripard" <mripard@kernel.org>
Cc: dri-devel@lists.freedesktop.org
Subject: Re: [PATCH v2 05/13] drm/tilcdc: Switch to drm_atomic_helper_crtc_create_state
Date: Mon, 31 Aug 2026 19:46:59 +0000	[thread overview]
Message-ID: <20260831194700.4B6541F000E9@smtp.kernel.org> (raw)
In-Reply-To: <20260831-drm-no-more-crtc-reset-v2-5-4500d49a337f@kernel.org>

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] Unconditional dereference of potentially NULL crtc->helper_private pointer when checking for the hw_reset hook.
--

commit 64d66423bd9e9cda8a92c70fb8fec4016d121ea0
Author: Maxime Ripard <mripard@kernel.org>

drm/tilcdc: Switch to drm_atomic_helper_crtc_create_state

The tilcdc crtc implementation uses the deprecated
drm_atomic_helper_crtc_reset() as its reset hook.

Switch to drm_atomic_helper_crtc_create_state() instead.

> diff --git a/drivers/gpu/drm/tilcdc/tilcdc_crtc.c b/drivers/gpu/drm/tilcdc/tilcdc_crtc.c
> index b299d73ce3856..f619b7a6e70d9 100644
> --- a/drivers/gpu/drm/tilcdc/tilcdc_crtc.c
> +++ b/drivers/gpu/drm/tilcdc/tilcdc_crtc.c
> @@ -714,7 +714,7 @@ static void tilcdc_crtc_hw_reset(struct drm_crtc *crtc)
>  static const struct drm_crtc_funcs tilcdc_crtc_funcs = {
>  	.set_config     = drm_atomic_helper_set_config,
>  	.page_flip      = drm_atomic_helper_page_flip,
> -	.reset		= drm_atomic_helper_crtc_reset,
> +	.atomic_create_state = drm_atomic_helper_crtc_create_state,

[Severity: High]
Does this change expose a NULL pointer dereference in the core DRM API?

By switching to .atomic_create_state, drm_mode_config_reset() will now call
drm_mode_config_crtc_reset_with_create_state() during driver probe or
system resume.

Looking at drm_mode_config_crtc_reset_with_create_state() in
drivers/gpu/drm/drm_mode_config.c:

drm_mode_config_crtc_reset_with_create_state() {
	const struct drm_crtc_helper_funcs *crtc_funcs = crtc->helper_private;
	...
	if (crtc_funcs->hw_reset)
		crtc_funcs->hw_reset(crtc);

	return 0;
}

The crtc_funcs pointer is dereferenced here without a preceding NULL guard. 
If a driver does not use CRTC helpers, crtc->helper_private will be NULL. 
Wouldn't this unconditional dereference to check for hw_reset lead to a 
crash for any driver leaving helper_private unpopulated?

>  	.atomic_duplicate_state = drm_atomic_helper_crtc_duplicate_state,
>  	.atomic_destroy_state = drm_atomic_helper_crtc_destroy_state,
>  	.enable_vblank	= tilcdc_crtc_enable_vblank,
>  	.disable_vblank	= tilcdc_crtc_disable_vblank,
>  };

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260831-drm-no-more-crtc-reset-v2-0-4500d49a337f@kernel.org?part=5

  reply	other threads:[~2026-08-31 19:47 UTC|newest]

Thread overview: 28+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-08-31 15:34 [PATCH v2 00/13] drm/crtc: Convert all drivers to atomic_create_state and remove reset Maxime Ripard
2026-08-31 15:34 ` [PATCH v2 01/13] drm/crtc: Introduce hw_reset helper hook Maxime Ripard
2026-08-31 18:34   ` sashiko-bot
2026-09-02  7:05   ` Thomas Zimmermann
2026-08-31 15:34 ` [PATCH v2 02/13] drm/amdgpu: vkms: Switch to drm_atomic_helper_crtc_create_state Maxime Ripard
2026-08-31 15:34 ` [PATCH v2 03/13] drm/logicvc: " Maxime Ripard
2026-08-31 15:34 ` [PATCH v2 04/13] drm/tilcdc: Move hardware reset to CRTC creation Maxime Ripard
2026-08-31 19:11   ` sashiko-bot
2026-09-02  7:07   ` Thomas Zimmermann
2026-08-31 15:34 ` [PATCH v2 05/13] drm/tilcdc: Switch to drm_atomic_helper_crtc_create_state Maxime Ripard
2026-08-31 19:46   ` sashiko-bot [this message]
2026-08-31 15:34 ` [PATCH v2 06/13] drm/atomic-helper: Remove drm_atomic_helper_crtc_reset Maxime Ripard
2026-08-31 15:34 ` [PATCH v2 07/13] drm/amdgpu: dm: Convert to atomic_create_state Maxime Ripard
2026-08-31 20:02   ` sashiko-bot
2026-08-31 15:34 ` [PATCH v2 08/13] drm/loongson: Move hardware reset to CRTC creation Maxime Ripard
2026-08-31 16:08   ` Icenowy Zheng
2026-08-31 20:13   ` sashiko-bot
2026-09-02  7:10   ` Thomas Zimmermann
2026-09-04  1:47     ` wuqianhai
2026-08-31 15:34 ` [PATCH v2 09/13] drm/loongson: Convert to atomic_create_state Maxime Ripard
2026-08-31 15:34 ` [PATCH v2 10/13] drm/mediatek: " Maxime Ripard
2026-08-31 15:34 ` [PATCH v2 11/13] drm/sitronix: st7920: " Maxime Ripard
2026-08-31 20:40   ` sashiko-bot
2026-09-01 11:05   ` Iker Pedrosa
2026-08-31 15:34 ` [PATCH v2 12/13] drm/atomic-helper: Remove __drm_atomic_helper_crtc_reset Maxime Ripard
2026-08-31 15:34 ` [PATCH v2 13/13] drm/crtc: Remove reset Maxime Ripard
2026-08-31 21:09   ` sashiko-bot
2026-09-02  7:11 ` [PATCH v2 00/13] drm/crtc: Convert all drivers to atomic_create_state and remove reset Thomas Zimmermann

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20260831194700.4B6541F000E9@smtp.kernel.org \
    --to=sashiko-bot@kernel.org \
    --cc=dri-devel@lists.freedesktop.org \
    --cc=mripard@kernel.org \
    --cc=sashiko-reviews@lists.linux.dev \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.