Received-SPF: pass (google.com: domain of linux-kernel+bounces-44816-linux.lists.archive=gmail.com@vger.kernel.org designates 139.178.88.99 as permitted sender) client-ip=139.178.88.99;
Date: Tue, 30 Jan 2024 10:16:13 -0500
From: Rodrigo Vivi <rodrigo.vivi@intel.com>
To: Johannes Berg <johannes@sipsolutions.net>
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>
Subject: Re: [PATCH 1/2] devcoredump: Remove devcoredump device if failing
 device is gone
Message-ID: <ZbkSvcEtRgTXuzgJ@intel.com>
References: <20240126151121.1076079-1-rodrigo.vivi@intel.com>
 <d57dc34fa8b0e25cec014b8001dcd0527d1c1013.camel@sipsolutions.net>
 <ZbgYyra1Ypa0nf6u@intel.com>
 <33df6c78c4c47a8f57a1c2bfe835065becb5a253.camel@sipsolutions.net>
Content-Type: text/plain; charset="us-ascii"
Content-Disposition: inline
In-Reply-To: <33df6c78c4c47a8f57a1c2bfe835065becb5a253.camel@sipsolutions.net>
Precedence: bulk
MIME-Version: 1.0
X-MS-Exchange-AntiSpam-MessageData-ChunkCount: 1
X-MS-Exchange-AntiSpam-MessageData-0: =?us-ascii?Q?lAwcM2sTbm8S/6cvv7XLAVNve7gZZ0app0S0e/kzBrQTmEpq9TEe99RvuLnJ?=
 =?us-ascii?Q?cgaAKwij9TitY5tbAXJfFPOyHuploOEDmpLMnwuMTWA1fCg4nwm3jKpTaJ4m?=
 =?us-ascii?Q?kA/IsKYnAqENdoE2aoPN3K7VDMPBpZH/4iGfnw7FHzIzPWD1lUsgsmfwLGOV?=
 =?us-ascii?Q?ejUIPloMFoOlOpRMcZBGqxFdZOmoRE+vJGJeJYF1MjNw6XKkxI9i5JgpN5tv?=
 =?us-ascii?Q?R238SZuXUATu3ZnaItetbqON+wZDt8VJW3fCcO+Wjmrhp4HJEsPLGiW8ABir?=
 =?us-ascii?Q?KTEuqrVO3gXvRep1Ml/Vfe/JYY+ZXFLBCp9CSnJ4vkHSiY+P+n22j11wSODE?=
 =?us-ascii?Q?31MEk3MDIQPAjHTCsNe0+XrTV6wgi5sej5sLMmYf9blbd9oQXvznCyy4p5Xn?=
 =?us-ascii?Q?5J0jqDPMD3kng4o0DHqV6xQkuLz8YsinQ+kdX6DGHLyzdRPo1HSHpMXqezLC?=
 =?us-ascii?Q?zv5oHncH608K6reeKrLRoNan+GENklGaT1P3jK5VrPYDbmk35e+PuYkAkiZp?=
 =?us-ascii?Q?3uHrVJLyKi1/N9lbBlRsQYqbiKangSWYFqQcCL90xtQAxYbj2aqU0Al7HSIo?=
 =?us-ascii?Q?WRarNcPsSnxvpJUJjzpqesEtRx7oecG4WnRHbSYBtGc4E6+T0iMs1jTCgVyD?=
 =?us-ascii?Q?Vn5lQOBcuZLWLlWjiOnZYDB4f1ChH9YRX8J6JOqr3Kn3DTvG4YnDZAAdcWm0?=
 =?us-ascii?Q?nMQW8FTEI9VZK2QZCpIINAEVeEc2ed4lyyiUSK9Mni3Cd1yUPrCFEz/ldLow?=
 =?us-ascii?Q?wWq9VW13p/Yvr63ZPJ2UwSFYbiCNgTMYvJg9o1IdGlPfGMiixZCWYeNF9KJV?=
 =?us-ascii?Q?RAISVlT+CwNYEiqlr41DsvUAB+7BXW3ifIh6CN4G6Eni3xYPKJlXp0WZ+9NR?=
 =?us-ascii?Q?3y2L44bXcjJPZgMB1+D5xDCQFHEfjZ87u6v/dnXfTvl8fwgrRGrgGye9axTE?=
 =?us-ascii?Q?eZC8PeqMl53h0dgPvlNJWKhoy65baSBOQkOR61OVt+dNeQf4oAruO4DqdKml?=
 =?us-ascii?Q?q4THcjpEQyrbvXhyYYvtdTl+4ueKMZaX41m/iMAehrQH3jTNHNW1esAQCFdq?=
 =?us-ascii?Q?dAGfF1llMSd1dgyibihNp3a7fHvg5GmW80yEKY6OgnJAu+i7dy6zEv6jbOe+?=
 =?us-ascii?Q?oiDZ1nCqa+gseGsFXsCebjelepVvCkC4OT2TjO50/xPiAXOhg5+c/OZSyn8D?=
 =?us-ascii?Q?CMuFzzC90UTADKDCCPCIbgToBo9/q/Sj+krzmDYEzC/lNdATWgnvzYw888aA?=
 =?us-ascii?Q?QNAnBvN1k7/UBf4lt+thgOmqx91TS1Ac0xIdMUEgmPt+2F/rjJUocfiLIpCY?=
 =?us-ascii?Q?fngi9LQ0CNhlTYRK9OautSW6ezO3aEDlam0gDajZmstE5ur6JIoXZDrhkB3e?=
 =?us-ascii?Q?RvVCbOpFSdOuvoQfprw8MbhoPb2BkpcSXbR69TGjX5id/KvJ+Y2NMntm75Jk?=
 =?us-ascii?Q?sXe+ywII2Oz/xXhqvmgFtB7Jrjs9+Lh5oJcuYoOcr1MI37eQJkQsi3Uut/Ii?=
 =?us-ascii?Q?gfekWYIoWkEnKtu/jzAtAtPlMf0ymEWLDpoumQ0e6bR0HlwaJCTeZCmBM2iz?=
 =?us-ascii?Q?wbrV30hUXJwc+J1IGlIrfPnoJLqZpjG9hP6J4o13?=
X-MS-Exchange-CrossTenant-Network-Message-Id: e1f0c879-b51f-4ba7-a654-08dc21a667d1
X-MS-Exchange-CrossTenant-AuthSource: MN0PR11MB6059.namprd11.prod.outlook.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 30 Jan 2024 15:16:17.2388
 (UTC)
X-MS-Exchange-CrossTenant-FromEntityHeader: Hosted
X-MS-Exchange-CrossTenant-Id: 46c98d88-e344-4ed4-8496-4ed7712e255d
X-MS-Exchange-CrossTenant-MailboxType: HOSTED
X-MS-Exchange-CrossTenant-UserPrincipalName: TpesBPD+WdtxClbRLUcxtPUG9nNSKRkUEmq0DXy/tOVTFPtRc5t7T9akjAMg2Rert/KZ2cNCtZASum3K0wMWBg==
X-MS-Exchange-Transport-CrossTenantHeadersStamped: PH0PR11MB5830
X-OriginatorOrg: intel.com

> > If the failing_device is gone, the 'data cookie' it used to register with
> > dev_coredumpm(... void *data,...), is also likely gone on a clean removal.
> 
> That's on the user. You'll always be able to shoot yourself in the foot.
> 
> > And to be honest, we shouldn't even count that the registered *read()
> > function pointer is valid anymore.
> 
> That's not true: the module cannot be removed, there's a reference to it
> if you're using dev_coredumpm() correctly (which is to say: pass
> THIS_MODULE to the struct module *owner argument).
> 
> > Well, we could indeed. And that would unblock our CI, but I'm afraid
> > it wouldn't protect the final user from bad memory access on a direct
> > $ cat /sys/class/devcoredump/devcd<n>/data
> > 
> > Shouldn't we consider this critical itself to justify this entirely
> > removal?
> 
> No? IMHO that's totally on the user. If you absolutely cannot make a
> standalone dump 'data' pointer (why not?! you can always stick the
> actual data into a vmalloc chunk and use dev_coredumpv()?)

hmm... fair enough. We would be okay here indeed since devcoredump always
free the data.

> then maybe we
> can offer ways of removing it when you need to?

well, to be honest my first local version was like that, offering
a dev_coredump_removal() that driver could request the removal
before going away.

> But I'd rather not, it
> feels weird to have a need for it.

We could change or CI and instruct our devs to always write
something to 'data' to ensure that devcoredump is deleted
before we can reload our module. Maybe that's the right
approach indeed, although I would really prefer to have
a direct way.


> 
> johannes