From e199188c24e0cea7f331d95ef08bfdc2179fe9fa Mon Sep 17 00:00:00 2001 From: Mulugeta Mammo Date: Tue, 28 Jul 2026 20:56:42 +0000 Subject: [PATCH 1/2] Fix gzopen/gzclose correctness issues - gzopen: check for open() failure (fd < 0) and gzdopen() failure (file == NULL) before registering the file in the map. Previously a failed open would pass fd=-1 to gzdopen, store a NULL-keyed map entry, and leak the fd on gzdopen failure. - gzclose: use off_t instead of int for the lseek return value to avoid truncation on files larger than 2 GiB. The truncated value caused truncate() to corrupt large files. Signed-off-by: Mulugeta Mammo --- zlib_accel.cpp | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/zlib_accel.cpp b/zlib_accel.cpp index f8d07b0..f77b73e 100644 --- a/zlib_accel.cpp +++ b/zlib_accel.cpp @@ -923,8 +923,14 @@ gzFile ZEXPORT gzopen(const char* path, const char* mode) { FileMode file_mode = FileMode::NONE; int oflag = GetOpenFlags(mode, &file_mode); int fd = open((const char*)path, oflag, 0666); + if (fd < 0) { + return nullptr; + } gzFile file = orig_gzdopen(fd, mode); - // TODO in case of error fall back to zlib and set execution path. + if (file == nullptr) { + close(fd); + return nullptr; + } Log(LogLevel::LOG_INFO, "gzopen Line ", __LINE__, ", file ", static_cast(file), ", path ", path, ", mode ", mode, "\n"); @@ -1337,7 +1343,7 @@ int ZEXPORT gzclose(gzFile file) { } // Capture file size and name before gzclose - int file_size = lseek(gz->fd, 0, SEEK_CUR); + off_t file_size = lseek(gz->fd, 0, SEEK_CUR); char file_path[MAXPATHLEN]; ssize_t readlink_ret = readlink(("/proc/self/fd/" + std::to_string(gz->fd)).c_str(), file_path, From 8b44479cfad13fed4b5f41066160b46456dc6b46 Mon Sep 17 00:00:00 2001 From: Olasoji Date: Thu, 30 Jul 2026 14:53:46 -0700 Subject: [PATCH 2/2] gzclose: unregister the file before closing it gzclose() called gzip_files.Unset(file) after orig_gzclose(file) had already freed the gzFile, on all three exit paths. The map is keyed by the gzFile address, so unsetting after the free can erase the entry of whatever file was allocated at that address in the meantime. Move the Unset to just after the lookup and drop the two trailing calls. The shared_ptr from ShardedMap::Get() keeps this file's state alive for the rest of the function. Defensive rather than a fix for an observed failure: under glibc a freed chunk returns to the freeing thread, so another thread's gzopen does not receive it. Allocators with cross-thread free lists offer no such guarantee, and unsetting once up front beats getting the ordering right on three exit paths. Signed-off-by: Olasoji --- zlib_accel.cpp | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/zlib_accel.cpp b/zlib_accel.cpp index f77b73e..7d15338 100644 --- a/zlib_accel.cpp +++ b/zlib_accel.cpp @@ -1329,6 +1329,12 @@ int ZEXPORT gzread(gzFile file, voidp buf, unsigned len) { int ZEXPORT gzclose(gzFile file) { auto gz = gzip_files.Get(file); + // Unregister up front, before orig_gzclose frees the gzFile. Unsetting after + // the free would erase the entry of whatever file has since been allocated at + // the same address, so do it once here rather than on each exit path. Holding + // gz (a shared_ptr) keeps this file's state alive until we return. + gzip_files.Unset(file); + Log(LogLevel::LOG_INFO, "gzclose Line ", __LINE__, ", file ", static_cast(file), ", buffered ", gz->data_buf_content, ", path ", static_cast(gz->path), "\n"); @@ -1350,7 +1356,6 @@ int ZEXPORT gzclose(gzFile file) { MAXPATHLEN - 1); if (readlink_ret == -1) { ret = orig_gzclose(file); - gzip_files.Unset(file); Log(LogLevel::LOG_ERROR, "gzclose Line ", __LINE__, ", readlink_ret return error \n"); return ret; @@ -1379,7 +1384,6 @@ int ZEXPORT gzclose(gzFile file) { Log(LogLevel::LOG_INFO, "gzclose Line ", __LINE__, ", file ", static_cast(file), ", return code ", ret, ", buffered processed ", gz->data_buf_pos, "\n"); - gzip_files.Unset(file); return ret; }