Received-SPF: pass (google.com: domain of linux-kernel+bounces-44878-linux.lists.archive=gmail.com@vger.kernel.org designates 147.75.48.161 as permitted sender) client-ip=147.75.48.161;
Message-ID: <d1ae193e9728583da3fbfac62ce1b20b0a34b60e.camel@sipsolutions.net>
Subject: Re: [PATCH 1/2] devcoredump: Remove devcoredump device if failing
 device is gone
From: Johannes Berg <johannes@sipsolutions.net>
To: Rodrigo Vivi <rodrigo.vivi@intel.com>
Cc: linux-kernel@vger.kernel.org, Jose Souza <jose.souza@intel.com>, Maarten
 Lankhorst <maarten.lankhorst@linux.intel.com>, Greg Kroah-Hartman
 <gregkh@linuxfoundation.org>,  "Rafael J . Wysocki" <rafael@kernel.org>
Date: Tue, 30 Jan 2024 16:51:57 +0100
In-Reply-To: <ZbkakWFtV5iQrfLP@intel.com>
References: <20240126151121.1076079-1-rodrigo.vivi@intel.com>
	 <d57dc34fa8b0e25cec014b8001dcd0527d1c1013.camel@sipsolutions.net>
	 <ZbgYyra1Ypa0nf6u@intel.com>
	 <33df6c78c4c47a8f57a1c2bfe835065becb5a253.camel@sipsolutions.net>
	 <ZbkSvcEtRgTXuzgJ@intel.com>
	 <8681168464fa85061db4a7234f89cead65cb0261.camel@sipsolutions.net>
	 <ZbkakWFtV5iQrfLP@intel.com>
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: quoted-printable
User-Agent: Evolution 3.50.3 (3.50.3-1.fc39) 
Precedence: bulk
MIME-Version: 1.0

On Tue, 2024-01-30 at 10:49 -0500, Rodrigo Vivi wrote:
>=20
> I will make changes to our driver to make the 'data' a standalone memory
> that devcoredump will free. this ensures no uaf and no null deref.
> data could be read even after unbinding the driver.
>=20
> What I meant to userspace 'writing to 'data'' was to ensure that
> on our CI we run something like
>=20
> if /sys/.../device/devcd<n> exists, then
> echo 1 > /sys/.../device/devcd<n>/data
> before attempting the rmmod <driver>
>=20
> our rmmod cannot get stuck or our CI is blocked, but then ensuring
> the devcd is gone with module_put happening is the only current way
> of not blocking the rmmod.
>=20

Ah, you were just concerned about the module removal, sure, that makes
sense.

Though depending on how you make that data pointer: if you just use
*sg() or *v() then you don't have this problem in the first place. OTOH,
it's probably good to have a udev rule to automatically capture the data
in CI anyway (and fail the test if it happened?)

johannes