[PATCH i-g-t 2/3] lib/igt_kmod: drop devcoredump before a PCI module unload

[Lucas De Marchi]

On Wed, Apr 03, 2024 at 11:14:08AM -0400, Rodrigo Vivi wrote:
>devcoredump holds a module reference, blocking the module removal.
>
>It is intentional from the devcoredump perspective to keep the
>log available even after the unbind/unprobe. However it blocks
>our module removal here.
>
>v2: Accepting many suggestions from Lucas.
>
>Cc: Lucas De Marchi <lucas.demarchi at intel.com>
>Cc: Maarten Lankhorst <maarten.lankhorst at linux.intel.com>
>Cc: José Roberto de Souza <jose.souza at intel.com>
>Signed-off-by: Rodrigo Vivi <rodrigo.vivi at intel.com>
>---
> lib/igt_kmod.c | 55 ++++++++++++++++++++++++++++++++++++++++++++++++++
> 1 file changed, 55 insertions(+)
>
>diff --git a/lib/igt_kmod.c b/lib/igt_kmod.c
>index cc242838f..14d51f4f6 100644
>--- a/lib/igt_kmod.c
>+++ b/lib/igt_kmod.c
>@@ -323,6 +323,59 @@ static int igt_kmod_unload_r(struct kmod_module *kmod, unsigned int flags)
> 	return err;
> }
>
>+static void igt_drop_devcoredump(const char *driver)
>+{
>+	char sysfspath[PATH_MAX];
>+	DIR *dir;
>+	char *devcoredump;
>+	FILE *data;
>+	struct dirent *entry;
>+	int len, ret;
>+
>+	len = snprintf(sysfspath, sizeof(sysfspath),
>+		       "/sys/bus/pci/drivers/%s", driver);
>+
>+	igt_assert(len < sizeof(sysfspath));
>+
>+	 /* Not a PCI module */
>+	if (access(sysfspath, F_OK))
>+		return;
>+
>+	devcoredump = sysfspath + len;
>+
>+	dir = opendir(sysfspath);
>+	igt_assert(dir);
>+
>+	while ((entry = readdir(dir)) != NULL) {
>+		if (entry->d_type != DT_LNK ||
>+		    strcmp(entry->d_name, ".") == 0 ||
>+		    strcmp(entry->d_name, "..") == 0)
>+			continue;
>+
>+		ret = snprintf(devcoredump, sizeof(sysfspath) - len,
>+			       "/%s/devcoredump", entry->d_name);

I  think this could be simplified a little bit further

		ret = snprintf(devcoredump, sizeof(sysfspath) - len,
			       "/%s/devcoredump/data", entry->d_name);
		igt_assert(ret < sizeof(sysfspath) - len);

		data = fopen(sysfspath, "w");
		if (data) {
			igt_info("Removing devcoredump before module unload: %s\n",
				 sysfspath);

			/*
			 * Write anything to devcoredump/data to
			 * force its deletion
			 */
			fprintf(data, "1\n");
			fclose(data);
		}

so it drops the TOCTOU of access()/open() and make it shorter.
... but totally optional. And untested).


Reviewed-by: Lucas De Marchi <lucas.demarchi at intel.com>

Lucas De Marchi