[PATCH v2] drm/client: Don't print a NULL firmware config mode

[PATCH v2] drm/client: Don't print a NULL firmware config mode

[Hugo Sales]

When some tiles of a tiled display are missing,
drm_client_firmware_config() replaces the connector's mode with
drm_connector_fallback_non_tiled_mode(). That returns NULL when
connector->modes has no mode other than the tile size, but the debug
message after it still reads modes[i]->name. Accessing a member through
a NULL pointer is undefined behaviour. Today printk happens to get an
address below PAGE_SIZE and prints "(efault)"

Print "(none)" as the mode name instead. A connector whose mode is NULL
is skipped by drm_client_modeset_probe(), so it is left off, as before

While at it, drop the extra colon in "[CONNECTOR::%d:%s]". Every other
DRM debug message prints [CONNECTOR:%d:%s], and the double colon breaks
grepping the log for a connector id

Sashiko found this while reviewing a series that makes the fallback
return NULL more often, by also skipping modes larger than the client

Fixes: cf1d0180f571 ("drm/fbdev: Fallback to non tiled mode if all tiles not present")
Link: https://lore.kernel.org/all/sashiko-outbox-173425@kernel.org/
Suggested-by: Thomas Zimmermann <tzimmermann@suse.de>
Signed-off-by: Hugo Sales <hugo@hsal.es>
---
Changes in v2:
- Use modes[i] ? modes[i]->name : "(none)" in the existing message
  instead of a second drm_dbg_kms() (Thomas)
- Drop the extra colon in "[CONNECTOR::%d:%s]" on the same line
- v1: https://lore.kernel.org/r/20261022114356.669988-1-hugo@hsal.es

I ran checkpatch and built drm_client_modeset.o with W=1

 drivers/gpu/drm/drm_client_modeset.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/drm_client_modeset.c b/drivers/gpu/drm/drm_client_modeset.c
index 0080a8e9556a..67f90e5d1512 100644
--- a/drivers/gpu/drm/drm_client_modeset.c
+++ b/drivers/gpu/drm/drm_client_modeset.c
@@ -754,10 +754,10 @@ static bool drm_client_firmware_config(struct drm_client_dev *client,
 		}
 		crtcs[i] = crtc;
 
-		drm_dbg_kms(dev, "[CONNECTOR::%d:%s] on [CRTC:%d:%s] using %s mode: %s\n",
+		drm_dbg_kms(dev, "[CONNECTOR:%d:%s] on [CRTC:%d:%s] using %s mode: %s\n",
 			    connector->base.id, connector->name,
 			    crtc->base.id, crtc->name,
-			    mode_type, modes[i]->name);
+			    mode_type, modes[i] ? modes[i]->name : "(none)");
 
 		fallback = false;
 		conn_configured |= BIT(i);
-- 
2.55.0

Re: [PATCH v2] drm/client: Don't print a NULL firmware config mode

[Thomas Zimmermann]

Hi

Am 23.10.26 um 13:21 schrieb Hugo Sales:
> When some tiles of a tiled display are missing,
> drm_client_firmware_config() replaces the connector's mode with
> drm_connector_fallback_non_tiled_mode(). That returns NULL when
> connector->modes has no mode other than the tile size, but the debug
> message after it still reads modes[i]->name. Accessing a member through
> a NULL pointer is undefined behaviour. Today printk happens to get an
> address below PAGE_SIZE and prints "(efault)"
>
> Print "(none)" as the mode name instead. A connector whose mode is NULL
> is skipped by drm_client_modeset_probe(), so it is left off, as before
>
> While at it, drop the extra colon in "[CONNECTOR::%d:%s]". Every other
> DRM debug message prints [CONNECTOR:%d:%s], and the double colon breaks
> grepping the log for a connector id
>
> Sashiko found this while reviewing a series that makes the fallback
> return NULL more often, by also skipping modes larger than the client
>
> Fixes: cf1d0180f571 ("drm/fbdev: Fallback to non tiled mode if all tiles not present")
> Link: https://lore.kernel.org/all/sashiko-outbox-173425@kernel.org/
> Suggested-by: Thomas Zimmermann <tzimmermann@suse.de>
> Signed-off-by: Hugo Sales <hugo@hsal.es>

Reviewed-by: Thomas Zimmermann <tzimmermann@suse.de>


> ---
> Changes in v2:
> - Use modes[i] ? modes[i]->name : "(none)" in the existing message
>    instead of a second drm_dbg_kms() (Thomas)
> - Drop the extra colon in "[CONNECTOR::%d:%s]" on the same line
> - v1: https://lore.kernel.org/r/20261022114356.669988-1-hugo@hsal.es
>
> I ran checkpatch and built drm_client_modeset.o with W=1
>
>   drivers/gpu/drm/drm_client_modeset.c | 4 ++--
>   1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/gpu/drm/drm_client_modeset.c b/drivers/gpu/drm/drm_client_modeset.c
> index 0080a8e9556a..67f90e5d1512 100644
> --- a/drivers/gpu/drm/drm_client_modeset.c
> +++ b/drivers/gpu/drm/drm_client_modeset.c
> @@ -754,10 +754,10 @@ static bool drm_client_firmware_config(struct drm_client_dev *client,
>   		}
>   		crtcs[i] = crtc;
>   
> -		drm_dbg_kms(dev, "[CONNECTOR::%d:%s] on [CRTC:%d:%s] using %s mode: %s\n",
> +		drm_dbg_kms(dev, "[CONNECTOR:%d:%s] on [CRTC:%d:%s] using %s mode: %s\n",
>   			    connector->base.id, connector->name,
>   			    crtc->base.id, crtc->name,
> -			    mode_type, modes[i]->name);
> +			    mode_type, modes[i] ? modes[i]->name : "(none)");
>   
>   		fallback = false;
>   		conn_configured |= BIT(i);

-- 
--
Thomas Zimmermann
Graphics Driver Developer
SUSE Software Solutions Germany GmbH
Frankenstr. 146, 90461 Nürnberg, Germany, www.suse.com
GF: Stefan Gaiser, Jochen Jaser, Abhinav Puri, (HRB 36809, AG Nürnberg)