From 90f7f7639e9cdc02d7de62f9fa2e72ef0712abbf Mon Sep 17 00:00:00 2001 From: Marcin Radomski Date: Tue, 18 Feb 2025 13:48:43 +0000 Subject: [PATCH 1/3] Prevent truncating log tag name The currently hard-coded limit is 23 characters. This limit originates from the pre-Android 7.0 32-character limit on system property names, because Android's `liblog` allows setting the log level at runtime through `log.tag` system properties [1]. The limit meant that "log.tag.SOMETAG" must have been shorter than 32 bytes, leaving 23 bytes for the tag itself. The system property length limitation was removed in [2], so since 2017 it no longer applies, and some Android components already use longer tags [3][4]. This change increases the size of the stack-allocated buffer used for null-terminating the tag to 128 bytes, and adds support for even longer tags by falling back to heap-allocated CString when the tag is longer than that. The fallback path will introduce a performance penalty, but at the same time will allow manual level adjustment via log.tag. system property regardless of the tag length. [1] https://cs.android.com/android/platform/superproject/main/+/main:system/logging/logd/README.property;l=50?q=f:readme.property [2] https://android-review.googlesource.com/c/platform/bionic/+/327226 [3] https://cs.android.com/android/platform/superproject/main/+/main:device/generic/goldfish-opengl/system/codecs/omx/plugin/GoldfishVideoDecoderOMXComponent.cpp;l=20 [4] https://cs.android.com/android/platform/superproject/main/+/main:hardware/interfaces/biometrics/fingerprint/2.2/default/BiometricsFingerprint.cpp;l=16 --- src/lib.rs | 20 +++++++++++++++----- 1 file changed, 15 insertions(+), 5 deletions(-) diff --git a/src/lib.rs b/src/lib.rs index 1403ccd..591ef33 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -179,7 +179,7 @@ impl AndroidLogger { static ANDROID_LOGGER: OnceLock = OnceLock::new(); -const LOGGING_TAG_MAX_LEN: usize = 23; +const LOGGING_TAG_MAX_LEN: usize = 128; const LOGGING_MSG_MAX_LEN: usize = 4000; impl Default for AndroidLogger { @@ -223,10 +223,20 @@ impl Log for AndroidLogger { .map(|s| s.as_bytes()) .unwrap_or_else(|| module_path.as_bytes()); - // truncate the tag here to fit into LOGGING_TAG_MAX_LEN - self.fill_tag_bytes(&mut tag_bytes, tag); - // use stack array as C string - let tag: &CStr = unsafe { CStr::from_ptr(mem::transmute(tag_bytes.as_ptr())) }; + // In case we end up allocating, keep the CString alive. + let mut _owned_tag = None; + let tag: &CStr = if tag.len() < tag_bytes.len() { + // use stack array as C string + self.fill_tag_bytes(&mut tag_bytes, tag); + // SAFETY: fill_tag_bytes always puts a nullbyte in tag_bytes. + unsafe { CStr::from_ptr(mem::transmute(tag_bytes.as_ptr())) } + } else { + // Tag longer than available stack buffer; allocate. + // SAFETY: if tag contains nullbytes, the Android logger will just ignore any + // characters that follow it. + _owned_tag = Some(unsafe { CString::from_vec_unchecked(tag.to_vec()) }); + _owned_tag.as_ref().unwrap() + }; // message must not exceed LOGGING_MSG_MAX_LEN // therefore split log message into multiple log calls From 020dedf1eddfdcdf678ee7f26bbcb51601741aad Mon Sep 17 00:00:00 2001 From: Marcin Radomski Date: Tue, 4 Mar 2025 11:15:35 +0000 Subject: [PATCH 2/3] Reduce LOGGING_TAG_MAX_LEN, add comments To use exactly 128-byte long stack allocated buffer. --- src/lib.rs | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/src/lib.rs b/src/lib.rs index 591ef33..0ba9ca4 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -179,7 +179,8 @@ impl AndroidLogger { static ANDROID_LOGGER: OnceLock = OnceLock::new(); -const LOGGING_TAG_MAX_LEN: usize = 128; +// Maximum length of a tag that does not require allocation. +const LOGGING_TAG_MAX_LEN: usize = 127; const LOGGING_MSG_MAX_LEN: usize = 4000; impl Default for AndroidLogger { @@ -211,7 +212,7 @@ impl Log for AndroidLogger { return; } - // tag must not exceed LOGGING_TAG_MAX_LEN + // tag longer than LOGGING_TAG_MAX_LEN causes allocation let mut tag_bytes: [MaybeUninit; LOGGING_TAG_MAX_LEN + 1] = uninit_array(); let module_path = record.module_path().unwrap_or_default().to_owned(); From e4da077617384078854a404c36ee73172f39e67e Mon Sep 17 00:00:00 2001 From: Marcin Radomski Date: Tue, 4 Mar 2025 11:21:06 +0000 Subject: [PATCH 3/3] Make allocating the tag safe code Also remove unnecessary to_owned call, and clean up _owned_tag use. --- src/lib.rs | 15 +++++++++------ 1 file changed, 9 insertions(+), 6 deletions(-) diff --git a/src/lib.rs b/src/lib.rs index 0ba9ca4..98a48d8 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -215,7 +215,7 @@ impl Log for AndroidLogger { // tag longer than LOGGING_TAG_MAX_LEN causes allocation let mut tag_bytes: [MaybeUninit; LOGGING_TAG_MAX_LEN + 1] = uninit_array(); - let module_path = record.module_path().unwrap_or_default().to_owned(); + let module_path = record.module_path().unwrap_or_default(); // If no tag was specified, use module name let custom_tag = &config.tag; @@ -225,7 +225,7 @@ impl Log for AndroidLogger { .unwrap_or_else(|| module_path.as_bytes()); // In case we end up allocating, keep the CString alive. - let mut _owned_tag = None; + let _owned_tag; let tag: &CStr = if tag.len() < tag_bytes.len() { // use stack array as C string self.fill_tag_bytes(&mut tag_bytes, tag); @@ -233,10 +233,13 @@ impl Log for AndroidLogger { unsafe { CStr::from_ptr(mem::transmute(tag_bytes.as_ptr())) } } else { // Tag longer than available stack buffer; allocate. - // SAFETY: if tag contains nullbytes, the Android logger will just ignore any - // characters that follow it. - _owned_tag = Some(unsafe { CString::from_vec_unchecked(tag.to_vec()) }); - _owned_tag.as_ref().unwrap() + // We're using either + // - CString::as_bytes on config.tag, or + // - str::as_bytes on record.module_path() + // Neither of those include the terminating nullbyte. + _owned_tag = CString::new(tag) + .expect("config.tag or record.module_path() should never contain nullbytes"); + _owned_tag.as_ref() }; // message must not exceed LOGGING_MSG_MAX_LEN