Linux-Hwmon Archive mirror
 help / color / mirror / Atom feed
From: Andi Shyti <andi.shyti@linux.intel.com>
To: Ashutosh Dixit <ashutosh.dixit@intel.com>
Cc: intel-gfx@lists.freedesktop.org,
	"Badal Nilawar" <badal.nilawar@intel.com>,
	"Ville Syrjälä" <ville.syrjala@linux.intel.com>,
	"Rodrigo Vivi" <rodrigo.vivi@intel.com>,
	"Jani Nikula" <jani.nikula@linux.intel.com>,
	linux-hwmon@vger.kernel.org, dri-devel@lists.freedesktop.org
Subject: Re: [PATCH] drm/i915/hwmon: Get rid of devm
Date: Thu, 18 Apr 2024 23:56:58 +0200	[thread overview]
Message-ID: <ZiGXKrheNEJlk56X@ashyti-mobl2.lan> (raw)
In-Reply-To: <20240417145646.793223-1-ashutosh.dixit@intel.com>

Hi Ashutosh,

On Wed, Apr 17, 2024 at 07:56:46AM -0700, Ashutosh Dixit wrote:
> When both hwmon and hwmon drvdata (on which hwmon depends) are device
> managed resources, the expectation, on device unbind, is that hwmon will be
> released before drvdata. However, in i915 there are two separate code
> paths, which both release either drvdata or hwmon and either can be
> released before the other. These code paths (for device unbind) are as
> follows (see also the bug referenced below):
> 
> Call Trace:
> release_nodes+0x11/0x70
> devres_release_group+0xb2/0x110
> component_unbind_all+0x8d/0xa0
> component_del+0xa5/0x140
> intel_pxp_tee_component_fini+0x29/0x40 [i915]
> intel_pxp_fini+0x33/0x80 [i915]
> i915_driver_remove+0x4c/0x120 [i915]
> i915_pci_remove+0x19/0x30 [i915]
> pci_device_remove+0x32/0xa0
> device_release_driver_internal+0x19c/0x200
> unbind_store+0x9c/0xb0
> 
> and
> 
> Call Trace:
> release_nodes+0x11/0x70
> devres_release_all+0x8a/0xc0
> device_unbind_cleanup+0x9/0x70
> device_release_driver_internal+0x1c1/0x200
> unbind_store+0x9c/0xb0
> 
> This means that in i915, if use devm, we cannot gurantee that hwmon will
> always be released before drvdata. Which means that we have a uaf if hwmon
> sysfs is accessed when drvdata has been released but hwmon hasn't.
> 
> The only way out of this seems to be do get rid of devm_ and release/free
> everything explicitly during device unbind.
> 
> v2: Change commit message and other minor code changes
> v3: Cleanup from i915_hwmon_register on error (Armin Wolf)
> v4: Eliminate potential static analyzer warning (Rodrigo)
>     Eliminate fetch_and_zero (Jani)
> v5: Restore previous logic for ddat_gt->hwmon_dev error return (Andi)

Thanks!

Reviewed-by: Andi Shyti <andi.shyti@linux.intel.com>

Andi

  reply	other threads:[~2024-04-18 21:57 UTC|newest]

Thread overview: 8+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2024-04-17 14:56 [PATCH] drm/i915/hwmon: Get rid of devm Ashutosh Dixit
2024-04-18 21:56 ` Andi Shyti [this message]
2024-04-19  1:05   ` Dixit, Ashutosh
  -- strict thread matches above, loose matches on Subject: below --
2024-04-16  3:55 Ashutosh Dixit
2024-04-13  0:10 Ashutosh Dixit
2024-04-13 14:43 ` Armin Wolf
2024-04-15 23:21   ` Dixit, Ashutosh
2024-04-14 23:23 ` Dixit, Ashutosh

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=ZiGXKrheNEJlk56X@ashyti-mobl2.lan \
    --to=andi.shyti@linux.intel.com \
    --cc=ashutosh.dixit@intel.com \
    --cc=badal.nilawar@intel.com \
    --cc=dri-devel@lists.freedesktop.org \
    --cc=intel-gfx@lists.freedesktop.org \
    --cc=jani.nikula@linux.intel.com \
    --cc=linux-hwmon@vger.kernel.org \
    --cc=rodrigo.vivi@intel.com \
    --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 read-only IMAP folder(s) and NNTP newsgroup(s).