linux-kernel - Re: [PATCH RESEND v2 06/32] drm/vkms: Introduce configfs for plane name

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

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <f2ee5ce4-418a-4170-9b0f-26bac570e72e@bootlin.com>
Date: Mon, 17 Nov 2025 09:56:48 +0000
From: Louis Chauvet <louis.chauvet@...tlin.com>
To: José Expósito <jose.exposito89@...il.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 RESEND v2 06/32] drm/vkms: Introduce configfs for plane
 name



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@...tlin.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
>>