linux-kernel - Re: [PATCH v15 01/19] drm/etnaviv: Implement drm_gem_object_funcs::print

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] [day] [month] [year] [list]

Message-ID: <a078e53c-bf2b-48f1-9ffb-43772d98b898@linux.dev>
Date: Sat, 9 Nov 2024 15:23:55 +0800
From: Sui Jingfeng <sui.jingfeng@...ux.dev>
To: Lucas Stach <l.stach@...gutronix.de>
Cc: Christian Gmeiner <christian.gmeiner@...il.com>,
 Russell King <linux+etnaviv@...linux.org.uk>,
 dri-devel@...ts.freedesktop.org, etnaviv@...ts.freedesktop.org,
 linux-kernel@...r.kernel.org
Subject: Re: [PATCH v15 01/19] drm/etnaviv: Implement
 drm_gem_object_funcs::print_info()

Hi,

On 2024/10/1 21:04, Lucas Stach wrote:
> Hi Sui,
>
> Am Sonntag, dem 08.09.2024 um 17:43 +0800 schrieb Sui Jingfeng:
>> It will be called by drm_gem_print_info() if implemented, and it can
>> provide more information about the framebuffer objects.
> Etnaviv GEM BOs are not framebuffer objects.
>
>> Signed-off-by: Sui Jingfeng <sui.jingfeng@...ux.dev>
>> ---
>>   drivers/gpu/drm/etnaviv/etnaviv_gem.c | 32 +++++++++++++++++++++++++++
>>   drivers/gpu/drm/etnaviv/etnaviv_gem.h |  2 +-
>>   2 files changed, 33 insertions(+), 1 deletion(-)
>>
>> diff --git a/drivers/gpu/drm/etnaviv/etnaviv_gem.c b/drivers/gpu/drm/etnaviv/etnaviv_gem.c
>> index 4247a10f8d4f..543d881585b3 100644
>> --- a/drivers/gpu/drm/etnaviv/etnaviv_gem.c
>> +++ b/drivers/gpu/drm/etnaviv/etnaviv_gem.c
>> @@ -534,8 +534,40 @@ static const struct vm_operations_struct vm_ops = {
>>   	.close = drm_gem_vm_close,
>>   };
>>   
>> +static const char *etnaviv_gem_obj_caching_info(u32 flags)
>> +{
>> +	switch (flags & ETNA_BO_CACHE_MASK) {
>> +	case ETNA_BO_CACHED:
>> +		return "cached";
>> +	case ETNA_BO_UNCACHED:
>> +		return "uncached";
>> +	case ETNA_BO_WC:
>> +		return "write-combine";
> "write-combined" to be consistent with the other two.


OK,


>> +	default:
>> +		break;
>> +	}
>> +
>> +	return "unknown";
>> +}
>> +
>> +static void etnaviv_gem_object_info(struct drm_printer *p,
>> +				    unsigned int indent,
>> +				    const struct drm_gem_object *obj)
>> +{
>> +	const struct etnaviv_gem_object *etnaviv_obj;
>> +
>> +	etnaviv_obj = container_of(obj, struct etnaviv_gem_object, base);
>> +
>> +	drm_printf_indent(p, indent, "caching mode=%s\n",
>> +			  etnaviv_gem_obj_caching_info(etnaviv_obj->flags));
>> +	drm_printf_indent(p, indent, "active=%s\n",
>> +			  str_yes_no(is_active(etnaviv_obj)));
>> +	drm_printf_indent(p, indent, "vaddr=%p\n", etnaviv_obj->vaddr);
> Why should we expose the vaddr to userspace? I don't see why this would
> be relevant even as debug info and it leaks the kernel vmap area
> address, which could be abused to facilitate kernel exploits.


This is nearly a re-implement for the etnaviv_gem_describe(),

It's not intend to leak, but to give us a *hint* that
if a specific buffer object have been VMAP-ed and
we will know how many BOs have been vmap-ed.


> Regards,
> Lucas
>
>> +}
>> +
>>   static const struct drm_gem_object_funcs etnaviv_gem_object_funcs = {
>>   	.free = etnaviv_gem_free_object,
>> +	.print_info = etnaviv_gem_object_info,
>>   	.pin = etnaviv_gem_prime_pin,
>>   	.unpin = etnaviv_gem_prime_unpin,
>>   	.get_sg_table = etnaviv_gem_prime_get_sg_table,
>> diff --git a/drivers/gpu/drm/etnaviv/etnaviv_gem.h b/drivers/gpu/drm/etnaviv/etnaviv_gem.h
>> index a42d260cac2c..3f8fe19a77cc 100644
>> --- a/drivers/gpu/drm/etnaviv/etnaviv_gem.h
>> +++ b/drivers/gpu/drm/etnaviv/etnaviv_gem.h
>> @@ -68,7 +68,7 @@ struct etnaviv_gem_ops {
>>   	int (*mmap)(struct etnaviv_gem_object *, struct vm_area_struct *);
>>   };
>>   
>> -static inline bool is_active(struct etnaviv_gem_object *etnaviv_obj)
>> +static inline bool is_active(const struct etnaviv_gem_object *etnaviv_obj)
>>   {
>>   	return atomic_read(&etnaviv_obj->gpu_active) != 0;
>>   }

-- 
Best regards,
Sui