All of lore.kernel.org
 help / color / mirror / Atom feed
From: Boris Brezillon <boris.brezillon@collabora.com>
To: Danilo Krummrich <dakr@redhat.com>
Cc: oe-kbuild-all@lists.linux.dev, dri-devel@lists.freedesktop.org,
	Boris Brezillon <bbrezillon@kernel.org>,
	kernel test robot <lkp@intel.com>
Subject: Re: [drm-misc:for-linux-next 2/2] drivers/gpu/drm/drm_debugfs.c:212:33: sparse: sparse: non size-preserving pointer to integer cast
Date: Wed, 26 Jul 2023 10:12:30 +0200	[thread overview]
Message-ID: <20230726101230.6e7d2eb1@collabora.com> (raw)
In-Reply-To: <9d49d9b1-1db8-63c9-b0f6-aa72904f6aaa@redhat.com>

On Wed, 26 Jul 2023 00:25:36 +0200
Danilo Krummrich <dakr@redhat.com> wrote:

> On 7/24/23 09:27, Boris Brezillon wrote:
> > On Fri, 21 Jul 2023 02:06:16 +0800
> > kernel test robot <lkp@intel.com> wrote:
> >   
> >> tree:   git://anongit.freedesktop.org/drm/drm-misc for-linux-next
> >> head:   c7a472297169156252a50d76965eb36b081186e2
> >> commit: 4f66feeab173bd73e71028b8c2e1dcea07e32dd5 [2/2] drm: debugfs: provide infrastructure to dump a DRM GPU VA space
> >> config: i386-randconfig-r092-20230720 (https://download.01.org/0day-ci/archive/20230721/202307210230.t2OnM5g0-lkp@intel.com/config)
> >> compiler: gcc-12 (Debian 12.2.0-14) 12.2.0
> >> reproduce: (https://download.01.org/0day-ci/archive/20230721/202307210230.t2OnM5g0-lkp@intel.com/reproduce)
> >>
> >> If you fix the issue in a separate patch/commit (i.e. not just a new version of
> >> the same patch/commit), kindly add following tags
> >> | Reported-by: kernel test robot <lkp@intel.com>
> >> | Closes: https://lore.kernel.org/oe-kbuild-all/202307210230.t2OnM5g0-lkp@intel.com/
> >>
> >> sparse warnings: (new ones prefixed by >>)  
> >>>> drivers/gpu/drm/drm_debugfs.c:212:33: sparse: sparse: non size-preserving pointer to integer cast  
> >>
> >> vim +212 drivers/gpu/drm/drm_debugfs.c
> >>
> >>     178	
> >>     179	/**
> >>     180	 * drm_debugfs_gpuva_info - dump the given DRM GPU VA space
> >>     181	 * @m: pointer to the &seq_file to write
> >>     182	 * @mgr: the &drm_gpuva_manager representing the GPU VA space
> >>     183	 *
> >>     184	 * Dumps the GPU VA mappings of a given DRM GPU VA manager.
> >>     185	 *
> >>     186	 * For each DRM GPU VA space drivers should call this function from their
> >>     187	 * &drm_info_list's show callback.
> >>     188	 *
> >>     189	 * Returns: 0 on success, -ENODEV if the &mgr is not initialized
> >>     190	 */
> >>     191	int drm_debugfs_gpuva_info(struct seq_file *m,
> >>     192				   struct drm_gpuva_manager *mgr)
> >>     193	{
> >>     194		struct drm_gpuva *va, *kva = &mgr->kernel_alloc_node;
> >>     195	
> >>     196		if (!mgr->name)
> >>     197			return -ENODEV;
> >>     198	
> >>     199		seq_printf(m, "DRM GPU VA space (%s) [0x%016llx;0x%016llx]\n",
> >>     200			   mgr->name, mgr->mm_start, mgr->mm_start + mgr->mm_range);
> >>     201		seq_printf(m, "Kernel reserved node [0x%016llx;0x%016llx]\n",
> >>     202			   kva->va.addr, kva->va.addr + kva->va.range);
> >>     203		seq_puts(m, "\n");
> >>     204		seq_puts(m, " VAs | start              | range              | end                | object             | object offset\n");
> >>     205		seq_puts(m, "-------------------------------------------------------------------------------------------------------------\n");
> >>     206		drm_gpuva_for_each_va(va, mgr) {
> >>     207			if (unlikely(va == kva))
> >>     208				continue;
> >>     209	
> >>     210			seq_printf(m, "     | 0x%016llx | 0x%016llx | 0x%016llx | 0x%016llx | 0x%016llx\n",
> >>     211				   va->va.addr, va->va.range, va->va.addr + va->va.range,  
> >>   > 212				   (u64)va->gem.obj, va->gem.offset);  
> > 
> > Oops, I didn't notice it when reviewing. You're leaking a kernel address
> > to user space here. You should probably use %p to print the GEM object
> > address, and add `no_hash_pointers` to your cmdline when you want to
> > debug things.  
> 
> %p doesn't really work well in terms of formatting, plus for debugfs I 
> thought this might be fine. I could maybe use ptr_to_hashval(), but then 
> 'no_hash_pointers' wouldn't do anything for it.

Right, it's probably fine for debugfs indeed. Guess the uintptr_t cast
Steve suggested is the right fix then.

WARNING: multiple messages have this Message-ID (diff)
From: Boris Brezillon <boris.brezillon@collabora.com>
To: Danilo Krummrich <dakr@redhat.com>
Cc: Boris Brezillon <bbrezillon@kernel.org>,
	kernel test robot <lkp@intel.com>,
	dri-devel@lists.freedesktop.org, oe-kbuild-all@lists.linux.dev
Subject: Re: [drm-misc:for-linux-next 2/2] drivers/gpu/drm/drm_debugfs.c:212:33: sparse: sparse: non size-preserving pointer to integer cast
Date: Wed, 26 Jul 2023 10:12:30 +0200	[thread overview]
Message-ID: <20230726101230.6e7d2eb1@collabora.com> (raw)
In-Reply-To: <9d49d9b1-1db8-63c9-b0f6-aa72904f6aaa@redhat.com>

On Wed, 26 Jul 2023 00:25:36 +0200
Danilo Krummrich <dakr@redhat.com> wrote:

> On 7/24/23 09:27, Boris Brezillon wrote:
> > On Fri, 21 Jul 2023 02:06:16 +0800
> > kernel test robot <lkp@intel.com> wrote:
> >   
> >> tree:   git://anongit.freedesktop.org/drm/drm-misc for-linux-next
> >> head:   c7a472297169156252a50d76965eb36b081186e2
> >> commit: 4f66feeab173bd73e71028b8c2e1dcea07e32dd5 [2/2] drm: debugfs: provide infrastructure to dump a DRM GPU VA space
> >> config: i386-randconfig-r092-20230720 (https://download.01.org/0day-ci/archive/20230721/202307210230.t2OnM5g0-lkp@intel.com/config)
> >> compiler: gcc-12 (Debian 12.2.0-14) 12.2.0
> >> reproduce: (https://download.01.org/0day-ci/archive/20230721/202307210230.t2OnM5g0-lkp@intel.com/reproduce)
> >>
> >> If you fix the issue in a separate patch/commit (i.e. not just a new version of
> >> the same patch/commit), kindly add following tags
> >> | Reported-by: kernel test robot <lkp@intel.com>
> >> | Closes: https://lore.kernel.org/oe-kbuild-all/202307210230.t2OnM5g0-lkp@intel.com/
> >>
> >> sparse warnings: (new ones prefixed by >>)  
> >>>> drivers/gpu/drm/drm_debugfs.c:212:33: sparse: sparse: non size-preserving pointer to integer cast  
> >>
> >> vim +212 drivers/gpu/drm/drm_debugfs.c
> >>
> >>     178	
> >>     179	/**
> >>     180	 * drm_debugfs_gpuva_info - dump the given DRM GPU VA space
> >>     181	 * @m: pointer to the &seq_file to write
> >>     182	 * @mgr: the &drm_gpuva_manager representing the GPU VA space
> >>     183	 *
> >>     184	 * Dumps the GPU VA mappings of a given DRM GPU VA manager.
> >>     185	 *
> >>     186	 * For each DRM GPU VA space drivers should call this function from their
> >>     187	 * &drm_info_list's show callback.
> >>     188	 *
> >>     189	 * Returns: 0 on success, -ENODEV if the &mgr is not initialized
> >>     190	 */
> >>     191	int drm_debugfs_gpuva_info(struct seq_file *m,
> >>     192				   struct drm_gpuva_manager *mgr)
> >>     193	{
> >>     194		struct drm_gpuva *va, *kva = &mgr->kernel_alloc_node;
> >>     195	
> >>     196		if (!mgr->name)
> >>     197			return -ENODEV;
> >>     198	
> >>     199		seq_printf(m, "DRM GPU VA space (%s) [0x%016llx;0x%016llx]\n",
> >>     200			   mgr->name, mgr->mm_start, mgr->mm_start + mgr->mm_range);
> >>     201		seq_printf(m, "Kernel reserved node [0x%016llx;0x%016llx]\n",
> >>     202			   kva->va.addr, kva->va.addr + kva->va.range);
> >>     203		seq_puts(m, "\n");
> >>     204		seq_puts(m, " VAs | start              | range              | end                | object             | object offset\n");
> >>     205		seq_puts(m, "-------------------------------------------------------------------------------------------------------------\n");
> >>     206		drm_gpuva_for_each_va(va, mgr) {
> >>     207			if (unlikely(va == kva))
> >>     208				continue;
> >>     209	
> >>     210			seq_printf(m, "     | 0x%016llx | 0x%016llx | 0x%016llx | 0x%016llx | 0x%016llx\n",
> >>     211				   va->va.addr, va->va.range, va->va.addr + va->va.range,  
> >>   > 212				   (u64)va->gem.obj, va->gem.offset);  
> > 
> > Oops, I didn't notice it when reviewing. You're leaking a kernel address
> > to user space here. You should probably use %p to print the GEM object
> > address, and add `no_hash_pointers` to your cmdline when you want to
> > debug things.  
> 
> %p doesn't really work well in terms of formatting, plus for debugfs I 
> thought this might be fine. I could maybe use ptr_to_hashval(), but then 
> 'no_hash_pointers' wouldn't do anything for it.

Right, it's probably fine for debugfs indeed. Guess the uintptr_t cast
Steve suggested is the right fix then.

  reply	other threads:[~2023-07-26  8:12 UTC|newest]

Thread overview: 8+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2023-07-20 18:06 [drm-misc:for-linux-next 2/2] drivers/gpu/drm/drm_debugfs.c:212:33: sparse: sparse: non size-preserving pointer to integer cast kernel test robot
2023-07-20 18:06 ` kernel test robot
2023-07-24  7:27 ` Boris Brezillon
2023-07-24  7:27   ` Boris Brezillon
2023-07-25 22:25   ` Danilo Krummrich
2023-07-25 22:25     ` Danilo Krummrich
2023-07-26  8:12     ` Boris Brezillon [this message]
2023-07-26  8:12       ` Boris Brezillon

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20230726101230.6e7d2eb1@collabora.com \
    --to=boris.brezillon@collabora.com \
    --cc=bbrezillon@kernel.org \
    --cc=dakr@redhat.com \
    --cc=dri-devel@lists.freedesktop.org \
    --cc=lkp@intel.com \
    --cc=oe-kbuild-all@lists.linux.dev \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.