Re: [PATCH v15 01/19] drm/etnaviv: Implement drm_gem_object_funcs::print_info()

[Lucas Stach]

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

> +	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.

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;
>  }