linux-kernel - Re: [PATCH v4 01/40] drm/gpuvm: Don't require obj lock in destructor path

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: <aDAVo-dle3wgFiJb@pollux>
Date: Fri, 23 May 2025 08:28:51 +0200
From: Danilo Krummrich <dakr@...nel.org>
To: Rob Clark <robdclark@...il.com>
Cc: Dave Airlie <airlied@...il.com>, dri-devel@...ts.freedesktop.org,
	freedreno@...ts.freedesktop.org, linux-arm-msm@...r.kernel.org,
	Connor Abbott <cwabbott0@...il.com>,
	Rob Clark <robdclark@...omium.org>,
	Maarten Lankhorst <maarten.lankhorst@...ux.intel.com>,
	Maxime Ripard <mripard@...nel.org>,
	Thomas Zimmermann <tzimmermann@...e.de>,
	Simona Vetter <simona@...ll.ch>,
	open list <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH v4 01/40] drm/gpuvm: Don't require obj lock in destructor
 path

On Thu, May 22, 2025 at 07:51:50PM -0700, Rob Clark wrote:
> So if you _really_ don't like the WEAK_REF flag, I have a workable alternative
> that addresses the performance problems.

The mode you want to introduce is broken, and I don't understand why you don't
want to accept that.

  1. It obviously breaks some features, which is why you have to add lots of
     WARN_ON() calls to the corresponding code paths, such that drivers won't
     call into them any more.
  2. It requires conditionals based on kref_read(), which is oviously racy, and
     can cause UAF bugs.
  3. I'm sure, if we look closely, we'll find more subtle bugs, because GPUVM
     was designed with a clear ownership and lifetime model, that this mode
     undermines entirely.

The only reason why your MSM implementation does not run into trouble is because
it upholds certain contitions such that the racy kref_read() code does not cause
issues.

So, we would need to document all those extra requirements that drivers would
need to uphold using this mode, which eliminates more perfectly normal use
cases, that people expect to just work, one example for that would be to have
the same GEM object in multiple VMs.

This would be a huge mess and a mode full of footguns for drivers, and hence a
NACK from my side.