On 11/13/25 14:21, José Expósito wrote:
> On Wed, Oct 29, 2025 at 03:36:43PM +0100, Louis Chauvet wrote:
>> Planes can have name, create a plane attribute to configure it. Currently
>> plane name is mainly used in logs.
>>
>> Signed-off-by: Louis Chauvet <louis.chauvet@bootlin.com>
>> ---
>> Documentation/gpu/vkms.rst | 3 ++-
>> drivers/gpu/drm/vkms/vkms_configfs.c | 32 ++++++++++++++++++++++++++++++++
>> 2 files changed, 34 insertions(+), 1 deletion(-)
>>
>> diff --git a/Documentation/gpu/vkms.rst b/Documentation/gpu/vkms.rst
>> index 3574e01b928d..1fe6e420c963 100644
>> --- a/Documentation/gpu/vkms.rst
>> +++ b/Documentation/gpu/vkms.rst
>> @@ -87,10 +87,11 @@ Start by creating one or more planes::
>>
>> sudo mkdir /config/vkms/my-vkms/planes/plane0
>>
>> -Planes have 1 configurable attribute:
>> +Planes have 2 configurable attributes:
>>
>> - type: Plane type: 0 overlay, 1 primary, 2 cursor (same values as those
>> exposed by the "type" property of a plane)
>> +- name: Name of the plane
>
> I'd like to mention again my comment on limiting the name to a set of
> well-known characters [1].
>
> 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 should avoid surprising user-space too much and allow
> only a set of "safe" characters.
>
> Maybe I'm too cautious, as this is valid code, but I'd like to bring up
> the discussion again to see if someone else agrees or disagrees.
>
> [1] https://lore.kernel.org/all/aPtgCUX5kixTh2ua@fedora/
Sorry, I completely forgot to send my mail drafts for your comments...
It was mainly "Will do for v2" except here:
For me this should not be a kernel concern, when the userspace read a
file/folder name, it can be anything, so the userspace should do the
proper sanitization.
For libinput it was "easy" to exploit because unauthenticated users can
create any device name, but for VKMS, you must already be a
"privilegied" user (can write to configfs). I don't see the added value
for a kernel-side limitation, it will be more code for almost no
security improvement.
If you really think this is important, do you know if the kernel have a
helper to do this kind of checks? I did not found anything in strings.h
and I don't want to implement it in VKMS.
>> Continue by creating one or more CRTCs::
>>
>> diff --git a/drivers/gpu/drm/vkms/vkms_configfs.c b/drivers/gpu/drm/vkms/vkms_configfs.c
>> index 07ab794e1052..be6c3ba998b9 100644
>> --- a/drivers/gpu/drm/vkms/vkms_configfs.c
>> +++ b/drivers/gpu/drm/vkms/vkms_configfs.c
>> @@ -322,10 +322,42 @@ static ssize_t plane_type_store(struct config_item *item, const char *page,
>> return (ssize_t)count;
>> }
>>
>> +static ssize_t plane_name_show(struct config_item *item, char *page)
>> +{
>> + struct vkms_configfs_plane *plane;
>> + const char *name;
>> +
>> + plane = plane_item_to_vkms_configfs_plane(item);
>> +
>> + scoped_guard(mutex, &plane->dev->lock)
>> + name = vkms_config_plane_get_name(plane->config);
>> +
>> + return sprintf(page, "%s\n", name);
>> +}
>> +
>> +static ssize_t plane_name_store(struct config_item *item, const char *page,
>> + size_t count)
>> +{
>> + struct vkms_configfs_plane *plane;
>> +
>> + plane = plane_item_to_vkms_configfs_plane(item);
>> +
>> + scoped_guard(mutex, &plane->dev->lock) {
>> + if (plane->dev->enabled)
>> + return -EBUSY;
>> +
>> + vkms_config_plane_set_name(plane->config, page);
>> + }
>> +
>> + return (ssize_t)count;
>> +}
>> +
>> CONFIGFS_ATTR(plane_, type);
>> +CONFIGFS_ATTR(plane_, name);
>>
>> static struct configfs_attribute *plane_item_attrs[] = {
>> &plane_attr_type,
>> + &plane_attr_name,
>> NULL,
>> };
>>
>>
>> --
>> 2.51.0
>>