This is an automated email from the ASF dual-hosted git repository.
laiyingchun pushed a commit to branch branch-1.17.x
in repository https://gitbox.apache.org/repos/asf/kudu.git
The following commit(s) were added to refs/heads/branch-1.17.x by this push:
new 126af8fd0 KUDU-3520 Fix file descriptor leak in encryption
126af8fd0 is described below
commit 126af8fd0f4d4c87af9fdf0eb806e5a77f60c756
Author: Attila Bukor <[email protected]>
AuthorDate: Mon Oct 30 09:42:08 2023 +0100
KUDU-3520 Fix file descriptor leak in encryption
In PosixEnv, when creating new file handles, file descriptors are
created first, which are then passed to file handle objects, which close
the file descriptors in their destructors. With encryption enabled,
things can go wrong before they're passed to these objects, in which
case the file descriptors can be leaked due to returning from these
methods without the file handle objects being created.
This commit fixes this leak by closing the files on the failure cases.
Unfortunately, I couldn't reproduce the bug reported originally after
several attempts with different fd limits as if it was set too low, it
failed with a different error, and if it was set higher, it just didn't
fail. It looks like this was an edge case triggered by an unhappy
coincidence of multiple variables.
Change-Id: I2412429d4fe836b705296e9e30453d7c4d030cec
Reviewed-on: http://gerrit.cloudera.org:8080/20631
Reviewed-by: Alexey Serbin <[email protected]>
Tested-by: Kudu Jenkins
Reviewed-on: http://gerrit.cloudera.org:8080/20717
---
src/kudu/util/env_posix.cc | 33 +++++++++++++++++++++++++++++++--
1 file changed, 31 insertions(+), 2 deletions(-)
diff --git a/src/kudu/util/env_posix.cc b/src/kudu/util/env_posix.cc
index b6cf3fcfb..007b923ae 100644
--- a/src/kudu/util/env_posix.cc
+++ b/src/kudu/util/env_posix.cc
@@ -1556,13 +1556,25 @@ class PosixEnv : public Env {
bool encrypted = opts.is_sensitive && IsEncryptionEnabled();
EncryptionHeader header;
if (encrypted) {
+ auto cleanup = MakeScopedCleanup([&]() {
+ fclose(f);
+ });
+
DCHECK(server_key_);
int fd;
RETURN_NOT_OK(DoOpen(fname, OpenMode::MUST_EXIST, &fd));
+
+ auto fd_cleanup = MakeScopedCleanup([&]() {
+ DoClose(fd);
+ });
+
RETURN_NOT_OK(ReadEncryptionHeader(fd, fname, *server_key_, &header));
if (fseek(f, kEncryptionHeaderSize, SEEK_CUR)) {
return IOError(fname, errno);
}
+
+ cleanup.cancel();
+ fd_cleanup.cancel();
}
result->reset(new PosixSequentialFile(fname, encrypted, f, header));
return Status::OK();
@@ -1588,7 +1600,7 @@ class PosixEnv : public Env {
bool encrypted = opts.is_sensitive && IsEncryptionEnabled();
if (encrypted) {
DCHECK(server_key_);
- RETURN_NOT_OK(ReadEncryptionHeader(fd, fname, *server_key_, &header));
+ RETURN_NOT_OK_EVAL(ReadEncryptionHeader(fd, fname, *server_key_,
&header), DoClose(fd));
}
result->reset(new PosixRandomAccessFile(fname, fd,
encrypted, header));
@@ -1606,7 +1618,12 @@ class PosixEnv : public Env {
TRACE_EVENT1("io", "PosixEnv::NewWritableFile", "path", fname);
int fd;
RETURN_NOT_OK(DoOpen(fname, opts.mode, &fd));
- return InstantiateNewWritableFile(fname, fd, opts, result);
+ auto cleanup = MakeScopedCleanup([&]() {
+ DoClose(fd);
+ });
+ RETURN_NOT_OK(InstantiateNewWritableFile(fname, fd, opts, result));
+ cleanup.cancel();
+ return Status::OK();
}
virtual Status NewTempWritableFile(const WritableFileOptions& opts,
@@ -1617,8 +1634,12 @@ class PosixEnv : public Env {
int fd = 0;
string tmp_filename;
RETURN_NOT_OK(MkTmpFile(name_template, &fd, &tmp_filename));
+ auto cleanup = MakeScopedCleanup([&]() {
+ DoClose(fd);
+ });
RETURN_NOT_OK(InstantiateNewWritableFile(tmp_filename, fd, opts, result));
created_filename->swap(tmp_filename);
+ cleanup.cancel();
return Status::OK();
}
@@ -1643,6 +1664,9 @@ class PosixEnv : public Env {
RETURN_NOT_OK(DoOpen(fname, opts.mode, &fd));
EncryptionHeader eh;
if (encrypt) {
+ auto cleanup = MakeScopedCleanup([&]() {
+ DoClose(fd);
+ });
DCHECK(server_key_);
if (size >= kEncryptionHeaderSize) {
RETURN_NOT_OK(ReadEncryptionHeader(fd, fname, *server_key_, &eh));
@@ -1650,6 +1674,7 @@ class PosixEnv : public Env {
RETURN_NOT_OK(GenerateHeader(&eh));
RETURN_NOT_OK(WriteEncryptionHeader(fd, fname, *server_key_, eh));
}
+ cleanup.cancel();
}
result->reset(new PosixRWFile(fname, fd, opts.sync_on_close,
encrypt, eh));
@@ -1664,9 +1689,13 @@ class PosixEnv : public Env {
bool encrypt = opts.is_sensitive && IsEncryptionEnabled();
EncryptionHeader eh;
if (encrypt) {
+ auto cleanup = MakeScopedCleanup([&]() {
+ DoClose(fd);
+ });
DCHECK(server_key_);
RETURN_NOT_OK(GenerateHeader(&eh));
RETURN_NOT_OK(WriteEncryptionHeader(fd, *created_filename, *server_key_,
eh));
+ cleanup.cancel();
}
res->reset(new PosixRWFile(*created_filename, fd, opts.sync_on_close,
encrypt, eh));
