linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Lyude Paul <lyude@redhat.com>
To: Sean Paul <sean@poorly.run>
Cc: dri-devel@lists.freedesktop.org, nouveau@lists.freedesktop.org,
	amd-gfx@lists.freedesktop.org, "Juston Li" <juston.li@intel.com>,
	"Imre Deak" <imre.deak@intel.com>,
	"Ville Syrjälä" <ville.syrjala@linux.intel.com>,
	"Harry Wentland" <hwentlan@amd.com>,
	"Daniel Vetter" <daniel.vetter@ffwll.ch>,
	"Maarten Lankhorst" <maarten.lankhorst@linux.intel.com>,
	"Maxime Ripard" <mripard@kernel.org>,
	"David Airlie" <airlied@linux.ie>,
	"Daniel Vetter" <daniel@ffwll.ch>,
	linux-kernel@vger.kernel.org
Subject: Re: [PATCH v2 26/27] drm/dp_mst: Also print unhashed pointers for malloc/topology references
Date: Wed, 09 Oct 2019 15:40:29 -0400	[thread overview]
Message-ID: <308f0fe3bf2705a975d4b1f9dfd16dc07724d454.camel@redhat.com> (raw)
In-Reply-To: <20190927142510.GS218215@art_vandelay>

Hey! Re: our discussion about this at XDC, I think I'm going to drop this
patch and just fix KASAN so it prints the hashed pointer as well, I'll cc you
on the patches for that as well

On Fri, 2019-09-27 at 10:25 -0400, Sean Paul wrote:
> On Tue, Sep 03, 2019 at 04:46:04PM -0400, Lyude Paul wrote:
> > Currently we only print mstb/port pointer addresses in our malloc and
> > topology refcount functions using the hashed-by-default %p, but
> > unfortunately if you're trying to debug a use-after-free error caused by
> > a refcounting error then this really isn't terribly useful. On the other
> > hand though, everything in the rest of the DP MST helpers uses hashed
> > pointer values as well and probably isn't useful to convert to unhashed.
> > So, let's just get the best of both worlds and print both the hashed and
> > unhashed pointer in our malloc/topology refcount debugging output. This
> > will hopefully make it a lot easier to figure out which port/mstb is
> > causing KASAN to get upset.
> > 
> > Cc: Juston Li <juston.li@intel.com>
> > Cc: Imre Deak <imre.deak@intel.com>
> > Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > Cc: Harry Wentland <hwentlan@amd.com>
> > Cc: Daniel Vetter <daniel.vetter@ffwll.ch>
> > Signed-off-by: Lyude Paul <lyude@redhat.com>
> 
> It's really too bad there isn't a CONFIG_DEBUG_SHOW_PK_ADDRESSES or even a
> value
> of kptr_restrict value that bypasses pointer hashing. I'm sure we're not the
> only ones to feel this pain. Maybe everyone just hacks vsnprintf...
> 
> As it is, I'm not totally sold on exposing the actual addresses
> unconditionally.
> What do you think about pulling the print out into a function and only
> printing
> px if a debug kconfig is set?
> 
> Sean
> 
> > ---
> >  drivers/gpu/drm/drm_dp_mst_topology.c | 34 ++++++++++++++++-----------
> >  1 file changed, 20 insertions(+), 14 deletions(-)
> > 
> > diff --git a/drivers/gpu/drm/drm_dp_mst_topology.c
> > b/drivers/gpu/drm/drm_dp_mst_topology.c
> > index 2fe24e366925..5b5c0b3b3c0e 100644
> > --- a/drivers/gpu/drm/drm_dp_mst_topology.c
> > +++ b/drivers/gpu/drm/drm_dp_mst_topology.c
> > @@ -1327,7 +1327,8 @@ static void
> >  drm_dp_mst_get_mstb_malloc(struct drm_dp_mst_branch *mstb)
> >  {
> >  	kref_get(&mstb->malloc_kref);
> > -	DRM_DEBUG("mstb %p (%d)\n", mstb, kref_read(&mstb->malloc_kref));
> > +	DRM_DEBUG("mstb %p/%px (%d)\n",
> > +		  mstb, mstb, kref_read(&mstb->malloc_kref));
> >  }
> >  
> >  /**
> > @@ -1344,7 +1345,8 @@ drm_dp_mst_get_mstb_malloc(struct drm_dp_mst_branch
> > *mstb)
> >  static void
> >  drm_dp_mst_put_mstb_malloc(struct drm_dp_mst_branch *mstb)
> >  {
> > -	DRM_DEBUG("mstb %p (%d)\n", mstb, kref_read(&mstb->malloc_kref) - 1);
> > +	DRM_DEBUG("mstb %p/%px (%d)\n",
> > +		  mstb, mstb, kref_read(&mstb->malloc_kref) - 1);
> >  	kref_put(&mstb->malloc_kref, drm_dp_free_mst_branch_device);
> >  }
> >  
> > @@ -1379,7 +1381,8 @@ void
> >  drm_dp_mst_get_port_malloc(struct drm_dp_mst_port *port)
> >  {
> >  	kref_get(&port->malloc_kref);
> > -	DRM_DEBUG("port %p (%d)\n", port, kref_read(&port->malloc_kref));
> > +	DRM_DEBUG("port %p/%px (%d)\n",
> > +		  port, port, kref_read(&port->malloc_kref));
> >  }
> >  EXPORT_SYMBOL(drm_dp_mst_get_port_malloc);
> >  
> > @@ -1396,7 +1399,8 @@ EXPORT_SYMBOL(drm_dp_mst_get_port_malloc);
> >  void
> >  drm_dp_mst_put_port_malloc(struct drm_dp_mst_port *port)
> >  {
> > -	DRM_DEBUG("port %p (%d)\n", port, kref_read(&port->malloc_kref) - 1);
> > +	DRM_DEBUG("port %p/%px (%d)\n",
> > +		  port, port, kref_read(&port->malloc_kref) - 1);
> >  	kref_put(&port->malloc_kref, drm_dp_free_mst_port);
> >  }
> >  EXPORT_SYMBOL(drm_dp_mst_put_port_malloc);
> > @@ -1447,8 +1451,8 @@ drm_dp_mst_topology_try_get_mstb(struct
> > drm_dp_mst_branch *mstb)
> >  	int ret = kref_get_unless_zero(&mstb->topology_kref);
> >  
> >  	if (ret)
> > -		DRM_DEBUG("mstb %p (%d)\n", mstb,
> > -			  kref_read(&mstb->topology_kref));
> > +		DRM_DEBUG("mstb %p/%px (%d)\n",
> > +			  mstb, mstb, kref_read(&mstb->topology_kref));
> >  
> >  	return ret;
> >  }
> > @@ -1471,7 +1475,8 @@ static void drm_dp_mst_topology_get_mstb(struct
> > drm_dp_mst_branch *mstb)
> >  {
> >  	WARN_ON(kref_read(&mstb->topology_kref) == 0);
> >  	kref_get(&mstb->topology_kref);
> > -	DRM_DEBUG("mstb %p (%d)\n", mstb, kref_read(&mstb->topology_kref));
> > +	DRM_DEBUG("mstb %p/%px (%d)\n",
> > +		  mstb, mstb, kref_read(&mstb->topology_kref));
> >  }
> >  
> >  /**
> > @@ -1489,8 +1494,8 @@ static void drm_dp_mst_topology_get_mstb(struct
> > drm_dp_mst_branch *mstb)
> >  static void
> >  drm_dp_mst_topology_put_mstb(struct drm_dp_mst_branch *mstb)
> >  {
> > -	DRM_DEBUG("mstb %p (%d)\n",
> > -		  mstb, kref_read(&mstb->topology_kref) - 1);
> > +	DRM_DEBUG("mstb %p/%px (%d)\n",
> > +		  mstb, mstb, kref_read(&mstb->topology_kref) - 1);
> >  	kref_put(&mstb->topology_kref, drm_dp_destroy_mst_branch_device);
> >  }
> >  
> > @@ -1546,8 +1551,8 @@ drm_dp_mst_topology_try_get_port(struct
> > drm_dp_mst_port *port)
> >  	int ret = kref_get_unless_zero(&port->topology_kref);
> >  
> >  	if (ret)
> > -		DRM_DEBUG("port %p (%d)\n", port,
> > -			  kref_read(&port->topology_kref));
> > +		DRM_DEBUG("port %p/%px (%d)\n",
> > +			  port, port, kref_read(&port->topology_kref));
> >  
> >  	return ret;
> >  }
> > @@ -1569,7 +1574,8 @@ static void drm_dp_mst_topology_get_port(struct
> > drm_dp_mst_port *port)
> >  {
> >  	WARN_ON(kref_read(&port->topology_kref) == 0);
> >  	kref_get(&port->topology_kref);
> > -	DRM_DEBUG("port %p (%d)\n", port, kref_read(&port->topology_kref));
> > +	DRM_DEBUG("port %p/%px (%d)\n",
> > +		  port, port, kref_read(&port->topology_kref));
> >  }
> >  
> >  /**
> > @@ -1585,8 +1591,8 @@ static void drm_dp_mst_topology_get_port(struct
> > drm_dp_mst_port *port)
> >   */
> >  static void drm_dp_mst_topology_put_port(struct drm_dp_mst_port *port)
> >  {
> > -	DRM_DEBUG("port %p (%d)\n",
> > -		  port, kref_read(&port->topology_kref) - 1);
> > +	DRM_DEBUG("port %p/%px (%d)\n",
> > +		  port, port, kref_read(&port->topology_kref) - 1);
> >  	kref_put(&port->topology_kref, drm_dp_destroy_port);
> >  }
> >  
> > -- 
> > 2.21.0
> > 
-- 
Cheers,
	Lyude Paul


  reply	other threads:[~2019-10-09 19:40 UTC|newest]

Thread overview: 62+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
     [not found] <20190903204645.25487-1-lyude@redhat.com>
2019-09-03 20:45 ` [PATCH v2 01/27] drm/dp_mst: Move link address dumping into a function Lyude Paul
2019-09-25 17:45   ` Sean Paul
2019-09-03 20:45 ` [PATCH v2 02/27] drm/dp_mst: Get rid of list clear in destroy_connector_work Lyude Paul
2019-09-25 17:45   ` Sean Paul
2019-09-03 20:45 ` [PATCH v2 03/27] drm/dp_mst: Destroy MSTBs asynchronously Lyude Paul
2019-09-25 18:16   ` Sean Paul
2019-09-25 20:08     ` Lyude Paul
2019-09-27 13:31       ` Sean Paul
2019-10-08  9:45         ` Daniel Vetter
2019-09-03 20:45 ` [PATCH v2 04/27] drm/dp_mst: Move test_calc_pbn_mode() into an actual selftest Lyude Paul
2019-09-25 18:17   ` Sean Paul
2019-09-03 20:45 ` [PATCH v2 05/27] drm/print: Add drm_err_printer() Lyude Paul
2019-09-03 20:45 ` [PATCH v2 06/27] drm/dp_mst: Combine redundant cases in drm_dp_encode_sideband_req() Lyude Paul
2019-09-03 21:35   ` Dave Airlie
2019-09-03 21:57   ` [PATCH v3] " Lyude Paul
2019-09-03 20:45 ` [PATCH v2 07/27] drm/dp_mst: Add sideband down request tracing + selftests Lyude Paul
2019-09-10  9:01   ` Jani Nikula
2019-09-03 20:45 ` [PATCH v2 08/27] drm/dp_mst: Remove PDT teardown in drm_dp_destroy_port() and refactor Lyude Paul
2019-09-25 19:00   ` Sean Paul
2019-09-03 20:45 ` [PATCH v2 09/27] drm/dp_mst: Refactor drm_dp_send_enum_path_resources Lyude Paul
2019-09-03 20:45 ` [PATCH v2 10/27] drm/dp_mst: Remove huge conditional in drm_dp_mst_handle_up_req() Lyude Paul
2019-09-03 20:45 ` [PATCH v2 11/27] drm/dp_mst: Constify guid in drm_dp_get_mst_branch_by_guid() Lyude Paul
2019-09-03 21:41   ` Dave Airlie
2019-09-03 20:45 ` [PATCH v2 12/27] drm/dp_mst: Refactor drm_dp_mst_handle_up_req() Lyude Paul
2019-09-03 20:45 ` [PATCH v2 13/27] drm/dp_mst: Refactor drm_dp_mst_handle_down_rep() Lyude Paul
2019-09-03 20:45 ` [PATCH v2 14/27] drm/dp_mst: Destroy topology_mgr mutexes Lyude Paul
2019-09-25 19:14   ` Sean Paul
2019-09-03 20:45 ` [PATCH v2 15/27] drm/dp_mst: Cleanup drm_dp_send_link_address() a bit Lyude Paul
2019-09-03 21:42   ` Dave Airlie
2019-09-03 20:45 ` [PATCH v2 16/27] drm/dp_mst: Refactor pdt setup/teardown, add more locking Lyude Paul
2019-09-25 19:27   ` Sean Paul
2019-09-25 21:00     ` Lyude Paul
2019-09-27 13:30       ` Sean Paul
2019-09-03 20:45 ` [PATCH v2 17/27] drm/dp_mst: Rename drm_dp_add_port and drm_dp_update_port Lyude Paul
2019-09-25 19:30   ` Sean Paul
2019-09-03 20:45 ` [PATCH v2 18/27] drm/dp_mst: Remove lies in {up,down}_rep_recv documentation Lyude Paul
2019-09-25 19:32   ` Sean Paul
2019-09-03 20:45 ` [PATCH v2 19/27] drm/dp_mst: Handle UP requests asynchronously Lyude Paul
2019-09-25 19:46   ` Sean Paul
2019-09-03 20:45 ` [PATCH v2 20/27] drm/dp_mst: Protect drm_dp_mst_port members with connection_mutex Lyude Paul
2019-09-25 20:00   ` Sean Paul
2019-09-25 21:01     ` Lyude Paul
2019-09-03 20:45 ` [PATCH v2 21/27] drm/dp_mst: Don't forget to update port->input in drm_dp_mst_handle_conn_stat() Lyude Paul
2019-09-25 20:03   ` Sean Paul
2019-09-03 20:46 ` [PATCH v2 22/27] drm/nouveau: Don't grab runtime PM refs for HPD IRQs Lyude Paul
2019-09-25 20:06   ` Sean Paul
2019-09-03 20:46 ` [PATCH v2 23/27] drm/amdgpu: Iterate through DRM connectors correctly Lyude Paul
2019-09-13 20:45   ` Alex Deucher
2019-09-27 13:48     ` Alex Deucher
2019-09-03 20:46 ` [PATCH v2 24/27] drm/amdgpu/dm: Resume short HPD IRQs before resuming MST topology Lyude Paul
2019-09-13 20:46   ` Alex Deucher
2019-09-25 20:08   ` Sean Paul
2019-09-25 21:52   ` [PATCH v4] " Lyude Paul
2019-09-27 13:29     ` Alex Deucher
2019-09-03 20:46 ` [PATCH v2 25/27] drm/dp_mst: Add basic topology reprobing when resuming Lyude Paul
2019-09-27 13:52   ` Sean Paul
2019-10-09 19:06     ` Lyude Paul
2019-09-03 20:46 ` [PATCH v2 26/27] drm/dp_mst: Also print unhashed pointers for malloc/topology references Lyude Paul
2019-09-27 14:25   ` Sean Paul
2019-10-09 19:40     ` Lyude Paul [this message]
2019-09-03 20:46 ` [PATCH v2 27/27] drm/dp_mst: Add topology ref history tracking for debugging Lyude Paul
2019-09-27 14:51   ` Sean Paul

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=308f0fe3bf2705a975d4b1f9dfd16dc07724d454.camel@redhat.com \
    --to=lyude@redhat.com \
    --cc=airlied@linux.ie \
    --cc=amd-gfx@lists.freedesktop.org \
    --cc=daniel.vetter@ffwll.ch \
    --cc=daniel@ffwll.ch \
    --cc=dri-devel@lists.freedesktop.org \
    --cc=hwentlan@amd.com \
    --cc=imre.deak@intel.com \
    --cc=juston.li@intel.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=maarten.lankhorst@linux.intel.com \
    --cc=mripard@kernel.org \
    --cc=nouveau@lists.freedesktop.org \
    --cc=sean@poorly.run \
    --cc=ville.syrjala@linux.intel.com \
    /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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).