diff mbox series

[kmod,01/13] libkmod: use a dup()'d fd for zlib

Message ID 20240212-decompression-fixes-v1-1-06f92ad07985@gmail.com (mailing list archive)
State New, archived
Headers show
Series Load compressed modules with compression-less kmod | expand

Commit Message

Emil Velikov via B4 Relay Feb. 12, 2024, 5:23 p.m. UTC
From: Emil Velikov <emil.l.velikov@gmail.com>

The gzdopen() API used, takes ownership of the fd. To make that more
explicit we clear it (-1) as applicable.

Yet again, kmod has explicit API to return the fd to the user - which
currently is used solely when uncompressed, so we're safe.

Regardless - simply duplicate the fd locally and use that with zlib.

Signed-off-by: Emil Velikov <emil.l.velikov@gmail.com>
---
 libkmod/libkmod-file.c | 19 ++++++++++++-------
 1 file changed, 12 insertions(+), 7 deletions(-)

Comments

Lucas De Marchi April 29, 2024, 11:13 p.m. UTC | #1
On Mon, Feb 12, 2024 at 05:23:02PM GMT, Emil Velikov via B4 Relay wrote:
>From: Emil Velikov <emil.l.velikov@gmail.com>
>
>The gzdopen() API used, takes ownership of the fd. To make that more
>explicit we clear it (-1) as applicable.
>
>Yet again, kmod has explicit API to return the fd to the user - which
>currently is used solely when uncompressed, so we're safe.
>
>Regardless - simply duplicate the fd locally and use that with zlib.
>
>Signed-off-by: Emil Velikov <emil.l.velikov@gmail.com>


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


Lucas De Marchi

>---
> libkmod/libkmod-file.c | 19 ++++++++++++-------
> 1 file changed, 12 insertions(+), 7 deletions(-)
>
>diff --git a/libkmod/libkmod-file.c b/libkmod/libkmod-file.c
>index b138e7e..b97062b 100644
>--- a/libkmod/libkmod-file.c
>+++ b/libkmod/libkmod-file.c
>@@ -317,12 +317,18 @@ static int load_zlib(struct kmod_file *file)
> 	int err = 0;
> 	off_t did = 0, total = 0;
> 	_cleanup_free_ unsigned char *p = NULL;
>+	int gzfd;
>
> 	errno = 0;
>-	file->gzf = gzdopen(file->fd, "rb");
>-	if (file->gzf == NULL)
>+	gzfd = fcntl(file->fd, F_DUPFD_CLOEXEC, 3);
>+	if (gzfd < 0)
>+		return -errno;
>+
>+	file->gzf = gzdopen(gzfd, "rb"); /* takes ownership of the fd */
>+	if (file->gzf == NULL) {
>+		close(gzfd);
> 		return -errno;
>-	file->fd = -1; /* now owned by gzf due gzdopen() */
>+	}
>
> 	for (;;) {
> 		int r;
>@@ -359,7 +365,7 @@ static int load_zlib(struct kmod_file *file)
> 	return 0;
>
> error:
>-	gzclose(file->gzf);
>+	gzclose(file->gzf); /* closes the gzfd */
> 	return err;
> }
>
>@@ -368,7 +374,7 @@ static void unload_zlib(struct kmod_file *file)
> 	if (file->gzf == NULL)
> 		return;
> 	free(file->memory);
>-	gzclose(file->gzf); /* closes file->fd */
>+	gzclose(file->gzf);
> }
>
> static const char magic_zlib[] = {0x1f, 0x8b};
>@@ -535,7 +541,6 @@ void kmod_file_unref(struct kmod_file *file)
> 	if (file->memory)
> 		file->ops->unload(file);
>
>-	if (file->fd >= 0)
>-		close(file->fd);
>+	close(file->fd);
> 	free(file);
> }
>
>-- 
>2.43.0
>
diff mbox series

Patch

diff --git a/libkmod/libkmod-file.c b/libkmod/libkmod-file.c
index b138e7e..b97062b 100644
--- a/libkmod/libkmod-file.c
+++ b/libkmod/libkmod-file.c
@@ -317,12 +317,18 @@  static int load_zlib(struct kmod_file *file)
 	int err = 0;
 	off_t did = 0, total = 0;
 	_cleanup_free_ unsigned char *p = NULL;
+	int gzfd;
 
 	errno = 0;
-	file->gzf = gzdopen(file->fd, "rb");
-	if (file->gzf == NULL)
+	gzfd = fcntl(file->fd, F_DUPFD_CLOEXEC, 3);
+	if (gzfd < 0)
+		return -errno;
+
+	file->gzf = gzdopen(gzfd, "rb"); /* takes ownership of the fd */
+	if (file->gzf == NULL) {
+		close(gzfd);
 		return -errno;
-	file->fd = -1; /* now owned by gzf due gzdopen() */
+	}
 
 	for (;;) {
 		int r;
@@ -359,7 +365,7 @@  static int load_zlib(struct kmod_file *file)
 	return 0;
 
 error:
-	gzclose(file->gzf);
+	gzclose(file->gzf); /* closes the gzfd */
 	return err;
 }
 
@@ -368,7 +374,7 @@  static void unload_zlib(struct kmod_file *file)
 	if (file->gzf == NULL)
 		return;
 	free(file->memory);
-	gzclose(file->gzf); /* closes file->fd */
+	gzclose(file->gzf);
 }
 
 static const char magic_zlib[] = {0x1f, 0x8b};
@@ -535,7 +541,6 @@  void kmod_file_unref(struct kmod_file *file)
 	if (file->memory)
 		file->ops->unload(file);
 
-	if (file->fd >= 0)
-		close(file->fd);
+	close(file->fd);
 	free(file);
 }