From 2d6887062b47b1d96f44e315a4ea4a6848cb316a Mon Sep 17 00:00:00 2001 From: "Jonathan M. Henson" Date: Thu, 16 Nov 2023 13:24:05 -0800 Subject: [PATCH 1/5] Fixed directory creation to ignore umask, and allow cross process lock to fall back to read only if it can't open the file in rw mode. --- source/posix/cross_process_lock.c | 38 ++++++++++++++++++++++++++++--- source/posix/file.c | 3 +++ 2 files changed, 38 insertions(+), 3 deletions(-) diff --git a/source/posix/cross_process_lock.c b/source/posix/cross_process_lock.c index 1e6db0bbb..ce9538bcd 100644 --- a/source/posix/cross_process_lock.c +++ b/source/posix/cross_process_lock.c @@ -8,6 +8,7 @@ #include #include #include +#include #include #include @@ -45,10 +46,12 @@ struct aws_cross_process_lock *aws_cross_process_lock_try_acquire( */ struct aws_byte_cursor path_prefix = aws_byte_cursor_from_c_str("/tmp/aws_crt_cross_process_lock/"); struct aws_string *path_to_create = aws_string_new_from_cursor(allocator, &path_prefix); - /* it's probably there already and we don't care if it is. The open will fail and we will handle it there + + /* it's probably there already and we don't care if it is. The open will fail, and we will handle it there * if we can't open it due to permissions. */ aws_directory_create(path_to_create); aws_string_destroy(path_to_create); + struct aws_byte_cursor path_suffix = aws_byte_cursor_from_c_str(".lock"); struct aws_byte_buf nonce_buf; @@ -59,16 +62,45 @@ struct aws_cross_process_lock *aws_cross_process_lock_try_acquire( struct aws_cross_process_lock *instance_lock = NULL; + errno = 0; int fd = open((const char *)nonce_buf.buffer, O_CREAT | O_RDWR, 0666); if (fd < 0) { - AWS_LOGF_ERROR( + AWS_LOGF_DEBUG( AWS_LS_COMMON_GENERAL, "static: Lock file %s failed to open with errno %d", (const char *)nonce_buf.buffer, errno); + aws_translate_and_raise_io_error_or(errno, AWS_ERROR_MUTEX_FAILED); - goto cleanup; + + if (aws_last_error() == AWS_ERROR_NO_PERMISSION) { + AWS_LOGF_DEBUG( + AWS_LS_COMMON_GENERAL, + "static: Lock file %s couldn't be opened due to file ownership permissions. Attempting to open as read " + "only", + (const char *)nonce_buf.buffer); + + errno = 0; + fd = open((const char *)nonce_buf.buffer, O_RDONLY); + + if (fd < 0) { + AWS_LOGF_ERROR( + AWS_LS_COMMON_GENERAL, + "static: Lock file %s failed to open with read-only permissions with errno %d", + (const char *)nonce_buf.buffer, + errno); + aws_translate_and_raise_io_error_or(errno, AWS_ERROR_MUTEX_FAILED); + goto cleanup; + } + } else { + AWS_LOGF_ERROR( + AWS_LS_COMMON_GENERAL, + "static: Lock file %s failed to open. The lock cannot be acquired.", + (const char *)nonce_buf.buffer); + goto cleanup; + } } + if (flock(fd, LOCK_EX | LOCK_NB) == -1) { AWS_LOGF_TRACE( AWS_LS_COMMON_GENERAL, diff --git a/source/posix/file.c b/source/posix/file.c index 868112955..6b9d21073 100644 --- a/source/posix/file.c +++ b/source/posix/file.c @@ -40,6 +40,9 @@ int aws_directory_create(const struct aws_string *dir_path) { return aws_translate_and_raise_io_error(errno_value); } + /* bypass umask by setting the perms we actually requested */ + chmod(aws_string_c_str(dir_path), S_IRWXU | S_IRWXG | S_IRWXO); + return AWS_OP_SUCCESS; } From 98188581d3af5f5c31100366a2375f73ceaacd14 Mon Sep 17 00:00:00 2001 From: "Jonathan M. Henson" Date: Thu, 16 Nov 2023 13:30:59 -0800 Subject: [PATCH 2/5] Remove /sys/stat as it's no longer needed. --- source/posix/cross_process_lock.c | 1 - 1 file changed, 1 deletion(-) diff --git a/source/posix/cross_process_lock.c b/source/posix/cross_process_lock.c index ce9538bcd..346340a06 100644 --- a/source/posix/cross_process_lock.c +++ b/source/posix/cross_process_lock.c @@ -8,7 +8,6 @@ #include #include #include -#include #include #include From e5d8838d43c2492000a8e0e20d1b511fd3ddab7e Mon Sep 17 00:00:00 2001 From: "Jonathan M. Henson" Date: Thu, 16 Nov 2023 13:38:19 -0800 Subject: [PATCH 3/5] Just do a dir existence check from the beginning for clarity and obviousness. --- source/posix/cross_process_lock.c | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/source/posix/cross_process_lock.c b/source/posix/cross_process_lock.c index 346340a06..775048f9b 100644 --- a/source/posix/cross_process_lock.c +++ b/source/posix/cross_process_lock.c @@ -46,9 +46,11 @@ struct aws_cross_process_lock *aws_cross_process_lock_try_acquire( struct aws_byte_cursor path_prefix = aws_byte_cursor_from_c_str("/tmp/aws_crt_cross_process_lock/"); struct aws_string *path_to_create = aws_string_new_from_cursor(allocator, &path_prefix); - /* it's probably there already and we don't care if it is. The open will fail, and we will handle it there - * if we can't open it due to permissions. */ - aws_directory_create(path_to_create); + /* It's probably there already and we don't care if it is. */ + if (!aws_directory_exists(path_to_create)) { + /* if this call fails just let it fail on open below. */ + aws_directory_create(path_to_create); + } aws_string_destroy(path_to_create); struct aws_byte_cursor path_suffix = aws_byte_cursor_from_c_str(".lock"); From a3e91d6d32760f2276f7732ff9babbf21cdfd308 Mon Sep 17 00:00:00 2001 From: "Jonathan M. Henson" Date: Thu, 16 Nov 2023 13:40:42 -0800 Subject: [PATCH 4/5] update comment. --- source/posix/cross_process_lock.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/source/posix/cross_process_lock.c b/source/posix/cross_process_lock.c index 775048f9b..41f389825 100644 --- a/source/posix/cross_process_lock.c +++ b/source/posix/cross_process_lock.c @@ -41,7 +41,7 @@ struct aws_cross_process_lock *aws_cross_process_lock_try_acquire( * The unix standard says /tmp has to be there and be writable. However, while it may be tempting to just use the * /tmp/ directory, it often has the sticky bit set which would prevent a subprocess from being able to call open * with create on the file. The solution is simple, just write it to a subdirectory inside - * /tmp using the same perms as the current process. + * /tmp using 0777 (default behavior for directory creation, as aws_directory_create() overrides umask). */ struct aws_byte_cursor path_prefix = aws_byte_cursor_from_c_str("/tmp/aws_crt_cross_process_lock/"); struct aws_string *path_to_create = aws_string_new_from_cursor(allocator, &path_prefix); From c137a09cd51e35f44172d442758528c38c82cb25 Mon Sep 17 00:00:00 2001 From: "Jonathan M. Henson" Date: Thu, 16 Nov 2023 13:47:16 -0800 Subject: [PATCH 5/5] Move the chmod call to where we actually need it for safety. --- source/posix/cross_process_lock.c | 5 ++++- source/posix/file.c | 3 --- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/source/posix/cross_process_lock.c b/source/posix/cross_process_lock.c index 41f389825..1ef5d2b5f 100644 --- a/source/posix/cross_process_lock.c +++ b/source/posix/cross_process_lock.c @@ -8,6 +8,7 @@ #include #include #include +#include #include #include @@ -41,7 +42,7 @@ struct aws_cross_process_lock *aws_cross_process_lock_try_acquire( * The unix standard says /tmp has to be there and be writable. However, while it may be tempting to just use the * /tmp/ directory, it often has the sticky bit set which would prevent a subprocess from being able to call open * with create on the file. The solution is simple, just write it to a subdirectory inside - * /tmp using 0777 (default behavior for directory creation, as aws_directory_create() overrides umask). + * /tmp and override umask via. chmod of 0777. */ struct aws_byte_cursor path_prefix = aws_byte_cursor_from_c_str("/tmp/aws_crt_cross_process_lock/"); struct aws_string *path_to_create = aws_string_new_from_cursor(allocator, &path_prefix); @@ -50,6 +51,8 @@ struct aws_cross_process_lock *aws_cross_process_lock_try_acquire( if (!aws_directory_exists(path_to_create)) { /* if this call fails just let it fail on open below. */ aws_directory_create(path_to_create); + /* bypass umask by setting the perms we actually requested */ + chmod(aws_string_c_str(path_to_create), S_IRWXU | S_IRWXG | S_IRWXO); } aws_string_destroy(path_to_create); diff --git a/source/posix/file.c b/source/posix/file.c index 6b9d21073..868112955 100644 --- a/source/posix/file.c +++ b/source/posix/file.c @@ -40,9 +40,6 @@ int aws_directory_create(const struct aws_string *dir_path) { return aws_translate_and_raise_io_error(errno_value); } - /* bypass umask by setting the perms we actually requested */ - chmod(aws_string_c_str(dir_path), S_IRWXU | S_IRWXG | S_IRWXO); - return AWS_OP_SUCCESS; }