Received-SPF: pass (google.com: domain of linux-kernel+bounces-29378-linux.lists.archive=gmail.com@vger.kernel.org designates 139.178.88.99 as permitted sender) client-ip=139.178.88.99;
From: Rodrigo Vivi <rodrigo.vivi@intel.com>
To: <linux-kernel@vger.kernel.org>
CC: Rodrigo Vivi <rodrigo.vivi@intel.com>, Jose Souza <jose.souza@intel.com>,
	Maarten Lankhorst <maarten.lankhorst@linux.intel.com>, Johannes Berg
	<johannes@sipsolutions.net>
Subject: [PATCH] devcoredump: Remove devcoredump device if failing device is gone
Date: Wed, 17 Jan 2024 14:53:49 -0500
Message-ID: <20240117195349.343083-1-rodrigo.vivi@intel.com>
Content-Transfer-Encoding: 8bit
Content-Type: text/plain
Precedence: bulk
MIME-Version: 1.0
X-MS-Exchange-AntiSpam-MessageData-ChunkCount: 1
X-MS-Exchange-AntiSpam-MessageData-0: =?us-ascii?Q?JrZs27Ny0i6U9BuxgIdy/J7Boh9OxGeenNh4VtzeKFRQLpoIEoZ8NjRF4+fQ?=
 =?us-ascii?Q?+tose+wI5fTV3JbQgpCpbrAdBFtNtZnXj4FK5uJewBi6r7XUWsmFYOurOtqh?=
 =?us-ascii?Q?3L2qpDZiIt6Mp9Xpy5LBCbjfD2F4LZvY/UEAgFWx8rk5wbJCWA63IX0JiodT?=
 =?us-ascii?Q?2fT15kvMEHF6fuhTvqCeqUc8hFfgTIHWwUmV2hpIIsoV6ebP+vHKmP7VFK0v?=
 =?us-ascii?Q?vIdB9xovD+AcNVLsTUwvdgpwPLlhXH7PTSncQIuBalyBxRW6cud6AVEnICb9?=
 =?us-ascii?Q?2gMS8sCoGpaKSS9+SB51YY34zM4UlI4yQ4RL6+/wDlEmOt+Y7E4ZLengpL5W?=
 =?us-ascii?Q?ZGMyoSvZ/AegrRUf56506ASqJhohTmrZg9u3blK9n62t8QzbkrPQ+6wZDNRu?=
 =?us-ascii?Q?6d0CF+gKMUNPuwOTEle65WeajHyp3xim8wJ87oc8Ok8vAoeJ9JReWootInMr?=
 =?us-ascii?Q?Nt/LNVQ077s2PLbyQx7IDs0sR5LZBvrRcLzoCvgMhimS3lTgnbyNl0bQO1Ci?=
 =?us-ascii?Q?EqEzh6zuW8vzdZr2y2LlRwCqoAVf034Ru7NVSir7mx2p7HZ3qn5VUIO9ydhC?=
 =?us-ascii?Q?q+2gqWYc99iHxFT7MSme8qsXrqKHHTSv2DFLkAou3KO0USDZ5GhKYEBPkhiN?=
 =?us-ascii?Q?OWXWAFbnVZA2zLOX8t3wjrE+G00MXQbFs+nGqdLxNINOZ3u4uRoqMo5xK1Co?=
 =?us-ascii?Q?KwmocWE/UtCk+eZo9ldd+bAc9aG/ZDAG76QYzBFG1E1gEw/2mr1Vn36a/bnP?=
 =?us-ascii?Q?Ds2e9QP2P9JQnV6oQSYCLkUj3e2g8j3QjL462a7As1KYvO3uNwk8w08d/ZHZ?=
 =?us-ascii?Q?CFtxVloCco7HBI/HUZpAUmM1GzVZZV59h/pMYl0jtN0der/BE/RF6/0vwvqQ?=
 =?us-ascii?Q?AIECdEhRGKdL9jVLSToBFFAipaIjf7emw0JyJGt9vHAJ8eIr0h86Pq7QCrjK?=
 =?us-ascii?Q?bH/QW59trUal+6s9YdEF6it6UXHRfPrnLCZzuSJSF4Cq8lcS18fnyRkSRJfZ?=
 =?us-ascii?Q?PrLE8psMCwdrEgmoMUNPFDgwLKN5KzkMNf7Cuvg+vBOqL04obMAyCMFn6Uhc?=
 =?us-ascii?Q?dY/We8lciNVZ8hV1l3ICetCMe184O1sbnTdWbcK0wfhrSzp/C82gJNHHlp8w?=
 =?us-ascii?Q?6de0DciVxHQcwi46FxGWFsKzDrp+qngyr16G+eNWPZlGqDnJBCcYe/LiiXpK?=
 =?us-ascii?Q?nJkLHzw1S2qdDhIPcyEjqYDxSzv44n/4DpufqqwVdS9M/sP5MjWkisIjK5J/?=
 =?us-ascii?Q?+7xOkfO7XCUzuTuMvdTttTSmvC6gP+9LvSKUzC18yeEwSgyGiMx+QwFcckiC?=
 =?us-ascii?Q?UEcB5eCULU6tGk+nBN2UDak4oTZDB3lBk8HaJgD2Isq/+DN/l48HikfuEbuR?=
 =?us-ascii?Q?1Kzq4l/jMUSVKkDrPsUGVGqaXfG2e4PVBh8pzOfHmf2gmhg31w+gn+AM35+u?=
 =?us-ascii?Q?5RhExNbRY5k0vrkwMFPQRs5H5Gy/DUb+u0uLr2c8qtESeRv0h314DqNTYpyj?=
 =?us-ascii?Q?EeOiXhRUDwstWeHznoQPFfk6EN6Wz9ccJjW5rne2EY66NDBQEVUQXn9pifJh?=
 =?us-ascii?Q?9ZUCnIfdMvin0soFwBsejpQ61mbJyIK6MJcTUu29?=
X-MS-Exchange-CrossTenant-Network-Message-Id: f10dd9ba-c041-43bc-cd58-08dc17960842
X-MS-Exchange-CrossTenant-AuthSource: MN0PR11MB6059.namprd11.prod.outlook.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 17 Jan 2024 19:53:53.2676
 (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: KgSrqZ8WLavII28Bt5Z7Sc0eVMvN2CWHwlF5Pr/07KX6EsaqTf7n+horeX+NycMmOkgCeL7pk3rAUHppE97Sag==
X-MS-Exchange-Transport-CrossTenantHeadersStamped: CO1PR11MB4884
X-OriginatorOrg: intel.com

Make dev_coredumpm a real device managed helper, that not only
frees the device after a scheduled delay (DEVCD_TIMEOUT), but
also when the failing/crashed device is gone.

The module remove for the drivers using devcoredump are currently
broken if attempted between the crash and the DEVCD_TIMEOUT, since
the symbolic sysfs link won't be deleted.

On top of that, for PCI devices, the unbind of the device will
call the pci .remove void function, that cannot fail. At that
time, our device is pretty much gone, but the read and free
functions are alive trough the devcoredump device and they
can get some NULL dereferences or use after free.

So, if the failing-device is gone, let's cancel the scheduled
work and remove devcoredump-device immediately.

Cc: Jose Souza <jose.souza@intel.com>
Cc: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
Cc: Johannes Berg <johannes@sipsolutions.net>
Signed-off-by: Rodrigo Vivi <rodrigo.vivi@intel.com>
---
 drivers/base/devcoredump.c | 29 ++++++++++++++++++++++-------
 1 file changed, 22 insertions(+), 7 deletions(-)

diff --git a/drivers/base/devcoredump.c b/drivers/base/devcoredump.c
index 7e2d1f0d903a..6db7a2fd9a02 100644
--- a/drivers/base/devcoredump.c
+++ b/drivers/base/devcoredump.c
@@ -8,6 +8,7 @@
 #include <linux/module.h>
 #include <linux/device.h>
 #include <linux/devcoredump.h>
+#include <linux/devm-helpers.h>
 #include <linux/list.h>
 #include <linux/slab.h>
 #include <linux/fs.h>
@@ -118,19 +119,24 @@ static ssize_t devcd_data_read(struct file *filp, struct kobject *kobj,
 	return devcd->read(buffer, offset, count, devcd->data, devcd->datalen);
 }
 
-static ssize_t devcd_data_write(struct file *filp, struct kobject *kobj,
-				struct bin_attribute *bin_attr,
-				char *buffer, loff_t offset, size_t count)
+static void devcd_remove_now(struct devcd_entry *devcd)
 {
-	struct device *dev = kobj_to_dev(kobj);
-	struct devcd_entry *devcd = dev_to_devcd(dev);
-
 	mutex_lock(&devcd->mutex);
 	if (!devcd->delete_work) {
 		devcd->delete_work = true;
 		mod_delayed_work(system_wq, &devcd->del_wk, 0);
 	}
 	mutex_unlock(&devcd->mutex);
+}
+
+static ssize_t devcd_data_write(struct file *filp, struct kobject *kobj,
+				struct bin_attribute *bin_attr,
+				char *buffer, loff_t offset, size_t count)
+{
+	struct device *dev = kobj_to_dev(kobj);
+	struct devcd_entry *devcd = dev_to_devcd(dev);
+
+	devcd_remove_now(devcd);
 
 	return count;
 }
@@ -304,6 +310,12 @@ static ssize_t devcd_read_from_sgtable(char *buffer, loff_t offset,
 				  offset);
 }
 
+static void devcd_remove(void *data)
+{
+	struct devcd_entry *devcd = data;
+	devcd_remove_now(devcd);
+}
+
 /**
  * dev_coredumpm - create device coredump with read/free methods
  * @dev: the struct device for the crashed device
@@ -379,7 +391,10 @@ void dev_coredumpm(struct device *dev, struct module *owner,
 
 	dev_set_uevent_suppress(&devcd->devcd_dev, false);
 	kobject_uevent(&devcd->devcd_dev.kobj, KOBJ_ADD);
-	INIT_DELAYED_WORK(&devcd->del_wk, devcd_del);
+	if (devm_add_action(dev, devcd_remove, devcd))
+		dev_warn(dev, "devcoredump managed auto-removal registration failed\n");
+	if (devm_delayed_work_autocancel(dev, &devcd->del_wk, devcd_del))
+		dev_warn(dev, "devcoredump managed autocancel work failed\n");
 	schedule_delayed_work(&devcd->del_wk, DEVCD_TIMEOUT);
 	mutex_unlock(&devcd->mutex);
 	return;
-- 
2.43.0