| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <aPtgCUX5kixTh2ua@fedora>
Date: Fri, 24 Oct 2025 13:16:25 +0200
From: José Expósito <jose.exposito89@...il.com>
To: Louis Chauvet <louis.chauvet@...tlin.com>
Cc: Haneen Mohammed <hamohammed.sa@...il.com>,
Simona Vetter <simona@...ll.ch>,
Melissa Wen <melissa.srw@...il.com>,
Maarten Lankhorst <maarten.lankhorst@...ux.intel.com>,
Maxime Ripard <mripard@...nel.org>,
Thomas Zimmermann <tzimmermann@...e.de>,
David Airlie <airlied@...il.com>, Jonathan Corbet <corbet@....net>,
victoria@...tem76.com, sebastian.wick@...hat.com,
thomas.petazzoni@...tlin.com, dri-devel@...ts.freedesktop.org,
linux-kernel@...r.kernel.org, linux-doc@...r.kernel.org
Subject: Re: [PATCH 01/22] drm/vkms: Introduce config for plane name
Hey Louis,
Thanks a lot for this series.
I'm reviewing it on my side and adding some KUnit tests to help
me with the review/testing process. I'll send the new tests once
they are ready :)
On Sat, Oct 18, 2025 at 04:01:01AM +0200, Louis Chauvet wrote:
> As planes can have a name in DRM, prepare VKMS to configure it using
> ConfigFS.
>
> Signed-off-by: Louis Chauvet <louis.chauvet@...tlin.com>
> ---
> drivers/gpu/drm/vkms/vkms_config.c | 4 ++++
> drivers/gpu/drm/vkms/vkms_config.h | 26 ++++++++++++++++++++++++++
> drivers/gpu/drm/vkms/vkms_drv.h | 5 +++--
> drivers/gpu/drm/vkms/vkms_output.c | 6 +-----
> drivers/gpu/drm/vkms/vkms_plane.c | 6 ++++--
> 5 files changed, 38 insertions(+), 9 deletions(-)
>
> diff --git a/drivers/gpu/drm/vkms/vkms_config.c b/drivers/gpu/drm/vkms/vkms_config.c
> index f8394a063ecf..ed172f800685 100644
> --- a/drivers/gpu/drm/vkms/vkms_config.c
> +++ b/drivers/gpu/drm/vkms/vkms_config.c
> @@ -350,6 +350,8 @@ static int vkms_config_show(struct seq_file *m, void *data)
> seq_puts(m, "plane:\n");
> seq_printf(m, "\ttype=%d\n",
> vkms_config_plane_get_type(plane_cfg));
> + seq_printf(m, "\tname=%s\n",
> + vkms_config_plane_get_name(plane_cfg));
> }
>
> vkms_config_for_each_crtc(vkmsdev->config, crtc_cfg) {
> @@ -390,6 +392,7 @@ struct vkms_config_plane *vkms_config_create_plane(struct vkms_config *config)
>
> plane_cfg->config = config;
> vkms_config_plane_set_type(plane_cfg, DRM_PLANE_TYPE_OVERLAY);
> + vkms_config_plane_set_name(plane_cfg, NULL);
> xa_init_flags(&plane_cfg->possible_crtcs, XA_FLAGS_ALLOC);
>
> list_add_tail(&plane_cfg->link, &config->planes);
> @@ -402,6 +405,7 @@ void vkms_config_destroy_plane(struct vkms_config_plane *plane_cfg)
> {
> xa_destroy(&plane_cfg->possible_crtcs);
> list_del(&plane_cfg->link);
> + kfree_const(plane_cfg->name);
> kfree(plane_cfg);
> }
> EXPORT_SYMBOL_IF_KUNIT(vkms_config_destroy_plane);
> diff --git a/drivers/gpu/drm/vkms/vkms_config.h b/drivers/gpu/drm/vkms/vkms_config.h
> index 4c8d668e7ef8..b69c35097ba0 100644
> --- a/drivers/gpu/drm/vkms/vkms_config.h
> +++ b/drivers/gpu/drm/vkms/vkms_config.h
> @@ -35,6 +35,7 @@ struct vkms_config {
> *
> * @link: Link to the others planes in vkms_config
> * @config: The vkms_config this plane belongs to
> + * @name: Name of the plane
> * @type: Type of the plane. The creator of configuration needs to ensures that
> * at least one primary plane is present.
> * @possible_crtcs: Array of CRTCs that can be used with this plane
> @@ -47,6 +48,7 @@ struct vkms_config_plane {
> struct list_head link;
> struct vkms_config *config;
>
> + const char *name;
> enum drm_plane_type type;
> struct xarray possible_crtcs;
>
> @@ -288,6 +290,30 @@ vkms_config_plane_set_type(struct vkms_config_plane *plane_cfg,
> plane_cfg->type = type;
> }
>
> +/**
> + * vkms_config_plane_set_name() - Set the plane name
> + * @plane_cfg: Plane to set the name to
> + * @name: New plane name. The name is copied.
> + */
> +static inline void
> +vkms_config_plane_set_name(struct vkms_config_plane *plane_cfg,
> + const char *name)
> +{
> + if (plane_cfg->name)
> + kfree_const(plane_cfg->name);
> + plane_cfg->name = kstrdup_const(name, GFP_KERNEL);
> +}
I think we should limit the name to a set of well-known charaters.
The reason is that, in libinput, we had a format string vulnerability
due to the kernel exposing devices with names containing strings like
"%s" in the name (CVE-2022-1215):
https://gitlab.freedesktop.org/libinput/libinput/-/issues/752
In my opinion, we could avoid surprising user-space too much and allow
only a set of "safe" characters.
> +/**
> + * vkms_config_plane_get_name - Get the plane name
Missing "()":
vkms_config_plane_get_name() - Get the plane name
> + * @plane_cfg: Plane to get the name from
> + */
> +static inline const char *
> +vkms_config_plane_get_name(struct vkms_config_plane *plane_cfg)
> +{
> + return plane_cfg->name;
> +}
> +
> /**
> * vkms_config_plane_attach_crtc - Attach a plane to a CRTC
> * @plane_cfg: Plane to attach
> diff --git a/drivers/gpu/drm/vkms/vkms_drv.h b/drivers/gpu/drm/vkms/vkms_drv.h
> index db260df1d4f6..9ad286f043b5 100644
> --- a/drivers/gpu/drm/vkms/vkms_drv.h
> +++ b/drivers/gpu/drm/vkms/vkms_drv.h
> @@ -225,6 +225,7 @@ struct vkms_output {
> };
>
> struct vkms_config;
> +struct vkms_config_plane;
>
> /**
> * struct vkms_device - Description of a VKMS device
> @@ -298,10 +299,10 @@ int vkms_output_init(struct vkms_device *vkmsdev);
> * vkms_plane_init() - Initialize a plane
> *
> * @vkmsdev: VKMS device containing the plane
> - * @type: type of plane to initialize
> + * @config: plane configuration
> */
> struct vkms_plane *vkms_plane_init(struct vkms_device *vkmsdev,
> - enum drm_plane_type type);
> + struct vkms_config_plane *config);
>
> /* CRC Support */
> const char *const *vkms_get_crc_sources(struct drm_crtc *crtc,
> diff --git a/drivers/gpu/drm/vkms/vkms_output.c b/drivers/gpu/drm/vkms/vkms_output.c
> index 2ee3749e2b28..22208d02afa4 100644
> --- a/drivers/gpu/drm/vkms/vkms_output.c
> +++ b/drivers/gpu/drm/vkms/vkms_output.c
> @@ -19,11 +19,7 @@ int vkms_output_init(struct vkms_device *vkmsdev)
> return -EINVAL;
>
> vkms_config_for_each_plane(vkmsdev->config, plane_cfg) {
> - enum drm_plane_type type;
> -
> - type = vkms_config_plane_get_type(plane_cfg);
> -
> - plane_cfg->plane = vkms_plane_init(vkmsdev, type);
> + plane_cfg->plane = vkms_plane_init(vkmsdev, plane_cfg);
> if (IS_ERR(plane_cfg->plane)) {
> DRM_DEV_ERROR(dev->dev, "Failed to init vkms plane\n");
> return PTR_ERR(plane_cfg->plane);
> diff --git a/drivers/gpu/drm/vkms/vkms_plane.c b/drivers/gpu/drm/vkms/vkms_plane.c
> index e592e47a5736..263376686794 100644
> --- a/drivers/gpu/drm/vkms/vkms_plane.c
> +++ b/drivers/gpu/drm/vkms/vkms_plane.c
> @@ -11,6 +11,7 @@
>
> #include "vkms_drv.h"
> #include "vkms_formats.h"
> +#include "vkms_config.h"
Nit: Includes are sorted alphabetically.
Jose
> static const u32 vkms_formats[] = {
> DRM_FORMAT_ARGB8888,
> @@ -217,7 +218,7 @@ static const struct drm_plane_helper_funcs vkms_plane_helper_funcs = {
> };
>
> struct vkms_plane *vkms_plane_init(struct vkms_device *vkmsdev,
> - enum drm_plane_type type)
> + struct vkms_config_plane *config)
> {
> struct drm_device *dev = &vkmsdev->drm;
> struct vkms_plane *plane;
> @@ -225,7 +226,8 @@ struct vkms_plane *vkms_plane_init(struct vkms_device *vkmsdev,
> plane = drmm_universal_plane_alloc(dev, struct vkms_plane, base, 0,
> &vkms_plane_funcs,
> vkms_formats, ARRAY_SIZE(vkms_formats),
> - NULL, type, NULL);
> + NULL, vkms_config_plane_get_type(config),
> + vkms_config_plane_get_name(config));
> if (IS_ERR(plane))
> return plane;
>
>
> --
> 2.51.0
>
Powered by blists - more mailing lists