linux-kernel - Re: [PATCH v2] misc: fastrpc: Fix channel resource access in device

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] [thread-next>] [day] [month] [year] [list]

Message-ID: <2025062434-reviving-grumble-1e53@gregkh>
Date: Tue, 24 Jun 2025 16:38:25 +0100
From: Greg KH <gregkh@...uxfoundation.org>
To: Dmitry Baryshkov <dmitry.baryshkov@....qualcomm.com>
Cc: Ekansh Gupta <ekansh.gupta@....qualcomm.com>,
	srinivas.kandagatla@...aro.org, linux-arm-msm@...r.kernel.org,
	quic_bkumar@...cinc.com, linux-kernel@...r.kernel.org,
	quic_chennak@...cinc.com, dri-devel@...ts.freedesktop.org,
	arnd@...db.de, stable@...nel.org
Subject: Re: [PATCH v2] misc: fastrpc: Fix channel resource access in
 device_open

On Tue, Jun 24, 2025 at 04:36:35PM +0100, Greg KH wrote:
> On Tue, Jun 24, 2025 at 04:27:21PM +0300, Dmitry Baryshkov wrote:
> > On Thu, Jun 19, 2025 at 10:40:26AM +0530, Ekansh Gupta wrote:
> > > During rpmsg_probe, fastrpc device nodes are created first, then
> > > channel specific resources are initialized, followed by
> > > of_platform_populate, which triggers context bank probing. This
> > > sequence can cause issues as applications might open the device
> > > node before channel resources are initialized or the session is
> > > available, leading to problems. For example, spin_lock is initialized
> > > after the device node creation, but it is used in device_open,
> > > potentially before initialization. Move device registration after
> > > channel resource initialization in fastrpc_rpmsg_probe.
> > 
> > You've moved device init, however there is still a possibility for the
> > context devices to be created, but not bound to the driver (because all
> > the probings are async). I think instead we should drop the extra
> > platform driver layer and create and set up corresponding devices
> > manually. For example, see how it is handled in
> > host1x_memory_context_list_init(). That function uses iommu-maps, but we
> > can use OF nodes and iommus instead.
> 
> Is this a real platform device?  If so, why do you need a second
> platform driver, what makes this so unique?  If this isn't a platform
> device, then why not just use the faux bus instead?
> 
> It seems that "number of sessions" is a DT property, is that something
> that is really defined by the hardware?  Or is it just a virtual thing
> that people are abusing in the DT?
> 
> And if you really have all these sessions, why not make them real
> devices, wouldn't that make things simpler?

Oh wait, these are "fake" platform devices under the parent (i.e. real)
platform device.  That's not good, please don't do that, use the faux
bus code now instead to properly handle this.  Attempting to create a
device when open() is called is really really odd...

thanks,

greg k-h