From 80bcff466da6a89f1374110a68152336a1181782 Mon Sep 17 00:00:00 2001 From: JIN Jie Date: Thu, 12 Sep 2024 15:11:53 +0800 Subject: [PATCH] fix: Crash caused by UnwindEntryShard on stack --- agent/Cargo.lock | 74 ++++++++++--------- agent/crates/trace-utils/src/unwind.rs | 71 ++++++++++++++---- agent/src/flow_generator/protocol_logs/tls.rs | 24 +++--- 3 files changed, 106 insertions(+), 63 deletions(-) diff --git a/agent/Cargo.lock b/agent/Cargo.lock index 19b72a760b6..050854a8b75 100644 --- a/agent/Cargo.lock +++ b/agent/Cargo.lock @@ -428,7 +428,7 @@ dependencies = [ "serde", "serde_bytes", "serde_json", - "time 0.3.21", + "time", "uuid", ] @@ -580,18 +580,17 @@ dependencies = [ [[package]] name = "chrono" -version = "0.4.26" +version = "0.4.38" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "ec837a71355b28f6556dbd569b37b3f363091c0bd4b2e735674521b4c5fd9bc5" +checksum = "a21f936df1771bf62b77f047b726c4625ff2e8aa607c01ec06e5a05bd8463401" dependencies = [ "android-tzdata", "iana-time-zone", "js-sys", "num-traits", "serde", - "time 0.1.45", "wasm-bindgen", - "winapi", + "windows-targets 0.52.5", ] [[package]] @@ -1056,7 +1055,7 @@ dependencies = [ "sysinfo", "tempfile", "thiserror", - "time 0.3.21", + "time", "tokio", "tonic", "tonic-build", @@ -1069,6 +1068,16 @@ dependencies = [ "zstd 0.13.2", ] +[[package]] +name = "deranged" +version = "0.3.11" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "b42b6fa04a440b495c8b04d0e71b707c585f83cb9cb28cf8cd0d976c315e31b4" +dependencies = [ + "powerfmt", + "serde", +] + [[package]] name = "derivative" version = "2.2.0" @@ -1359,7 +1368,7 @@ dependencies = [ "regex", "rustversion", "thiserror", - "time 0.3.21", + "time", ] [[package]] @@ -1532,7 +1541,7 @@ checksum = "c85e1d9ab2eadba7e5040d4e09cbd6d072b76a557ad64e797c2cb9d4da21d7e4" dependencies = [ "cfg-if", "libc", - "wasi 0.11.0+wasi-snapshot-preview1", + "wasi", ] [[package]] @@ -2463,7 +2472,7 @@ source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "a4a650543ca06a924e8b371db273b2756685faae30f8487da1b56505a8f78b0c" dependencies = [ "libc", - "wasi 0.11.0+wasi-snapshot-preview1", + "wasi", "windows-sys 0.48.0", ] @@ -2558,6 +2567,12 @@ dependencies = [ "winapi", ] +[[package]] +name = "num-conv" +version = "0.1.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "51d515d32fb182ee37cda2ccdcb92950d6a3c2893aa280e540671c2cd0f3b1d9" + [[package]] name = "num-traits" version = "0.2.15" @@ -2998,6 +3013,12 @@ dependencies = [ "pnet_sys", ] +[[package]] +name = "powerfmt" +version = "0.2.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "439ee305def115ba05938db6eb1644ff94165c5ab5e9420d1c1bcedbba909391" + [[package]] name = "ppv-lite86" version = "0.2.17" @@ -3712,7 +3733,7 @@ dependencies = [ "serde", "serde_derive", "serde_json", - "time 0.3.21", + "time", ] [[package]] @@ -3976,24 +3997,16 @@ dependencies = [ [[package]] name = "time" -version = "0.1.45" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "1b797afad3f312d1c66a56d11d0316f916356d11bd158fbc6ca6389ff6bf805a" -dependencies = [ - "libc", - "wasi 0.10.0+wasi-snapshot-preview1", - "winapi", -] - -[[package]] -name = "time" -version = "0.3.21" +version = "0.3.36" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "8f3403384eaacbca9923fa06940178ac13e4edb725486d70e8e15881d0c836cc" +checksum = "5dfd88e563464686c916c7e46e623e520ddc6d79fa6641390f2e3fa86e83e885" dependencies = [ + "deranged", "itoa", "libc", + "num-conv", "num_threads", + "powerfmt", "serde", "time-core", "time-macros", @@ -4001,16 +4014,17 @@ dependencies = [ [[package]] name = "time-core" -version = "0.1.1" +version = "0.1.2" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "7300fbefb4dadc1af235a9cef3737cea692a9d97e1b9cbcd4ebdae6f8868e6fb" +checksum = "ef927ca75afb808a4d64dd374f00a2adf8d0fcff8e7b184af886c3c87ec4a3f3" [[package]] name = "time-macros" -version = "0.2.9" +version = "0.2.18" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "372950940a5f07bf38dbe211d7283c9e6d7327df53794992d293e534c733d09b" +checksum = "3f252a68540fde3a3877aeea552b832b40ab9a69e318efd078774a01ddee1ccf" dependencies = [ + "num-conv", "time-core", ] @@ -4455,12 +4469,6 @@ dependencies = [ "try-lock", ] -[[package]] -name = "wasi" -version = "0.10.0+wasi-snapshot-preview1" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "1a143597ca7c7793eff794def352d41792a93c481eb1042423ff7ff72ba2c31f" - [[package]] name = "wasi" version = "0.11.0+wasi-snapshot-preview1" diff --git a/agent/crates/trace-utils/src/unwind.rs b/agent/crates/trace-utils/src/unwind.rs index 1296dd1817d..8ac2d1b9532 100644 --- a/agent/crates/trace-utils/src/unwind.rs +++ b/agent/crates/trace-utils/src/unwind.rs @@ -17,11 +17,13 @@ pub mod dwarf; pub mod maps; +use std::alloc::{alloc, dealloc, handle_alloc_error, Layout}; use std::collections::{hash_map::Entry, HashMap, HashSet, VecDeque}; use std::fs; use std::hash::Hasher; use std::mem; use std::path::PathBuf; +use std::ptr::NonNull; use std::slice; use ahash::AHasher; @@ -69,7 +71,7 @@ impl UnwindTable { trace!("load dwarf entries for process#{pid}"); let mut shard_list = ProcessShardList::default(); - let mut shard: Option = None; + let mut shard: Option = None; let mut shard_count = 0; let base_path: PathBuf = ["/proc", &pid.to_string(), "root"].iter().collect(); @@ -168,7 +170,7 @@ impl UnwindTable { continue; } - match shard.as_ref() { + match shard.as_ref().map(|b| b.as_ref()) { Some(s) if s.len as usize + entries.len() > UNWIND_ENTRIES_PER_SHARD => { trace!( "finish shard#{} because {} + {} > {}", @@ -177,19 +179,20 @@ impl UnwindTable { entries.len(), UNWIND_ENTRIES_PER_SHARD ); - let s = shard.take().unwrap(); - self.update_unwind_entry_shard(s.id, &s); + let boxed_shard = shard.take().unwrap(); + let s = boxed_shard.as_ref(); + self.update_unwind_entry_shard(s.id, s); } _ => (), } if shard.is_none() { let shard_id = self.id_gen.acquire(); trace!("create shard#{shard_id} for unwind entries"); - shard.replace(UnwindEntryShard::new(shard_id)); + shard.replace(BoxedShard::new(shard_id)); shard_count += 1; } - let shard = shard.as_mut().unwrap(); + let shard = shard.as_mut().unwrap().as_mut(); trace!( "load object {} into shard#{} with {} entries", path.display(), @@ -230,9 +233,10 @@ impl UnwindTable { ); shard_list.len += 1; } - if let Some(s) = shard.take() { + if let Some(bs) = shard.take() { + let s = bs.as_ref(); trace!("finish shard#{} with {} entries", s.id, s.len); - self.update_unwind_entry_shard(s.id, &s); + self.update_unwind_entry_shard(s.id, s); } if shard_list.len == 0 { @@ -346,6 +350,7 @@ impl UnwindTable { ..Default::default() }; let mut first_shard = true; + let mut boxed_shard = BoxedShard::new(0); for chunk in entries.chunks(UNWIND_ENTRIES_PER_SHARD) { let shard_id = self.id_gen.acquire(); trace!( @@ -353,11 +358,12 @@ impl UnwindTable { chunk.len() ); - let mut shard = UnwindEntryShard::new(shard_id); + let shard = boxed_shard.as_mut(); + shard.id = shard_id; shard.len = chunk.len() as u32; (&mut shard.entries[..shard.len as usize]).copy_from_slice(chunk); - self.update_unwind_entry_shard(shard_id, &shard); + self.update_unwind_entry_shard(shard_id, shard); if shard_list.len as usize >= UNWIND_SHARDS_PER_PROCESS { warn!( @@ -565,6 +571,9 @@ impl Default for ProcessShardList { } } +/* + * This struct has size of 1M, do not allocate it on stack + */ #[repr(C)] #[derive(Clone, Debug)] pub struct UnwindEntryShard { @@ -573,12 +582,44 @@ pub struct UnwindEntryShard { pub entries: [UnwindEntry; UNWIND_ENTRIES_PER_SHARD], } -impl UnwindEntryShard { +struct BoxedShard(NonNull); + +impl BoxedShard { fn new(id: u32) -> Self { - Self { - id, - len: 0, - entries: [UnwindEntry::default(); UNWIND_ENTRIES_PER_SHARD], + // creating UnwindEntryShard with `new` or `default` still use stack + // allocate with Allocater API to avoid this + unsafe { + let layout = Layout::new::(); + let ptr = alloc(layout); + if ptr.is_null() { + handle_alloc_error(layout); + } + let ptr = ptr as *mut UnwindEntryShard; + (*ptr).id = id; + (*ptr).len = 0; + // entries array do not require initializing + Self(NonNull::new_unchecked(ptr as *mut UnwindEntryShard)) + } + } +} + +impl Drop for BoxedShard { + fn drop(&mut self) { + unsafe { + let layout = Layout::new::(); + dealloc(self.0.as_ptr() as *mut u8, layout); } } } + +impl AsRef for BoxedShard { + fn as_ref(&self) -> &UnwindEntryShard { + unsafe { self.0.as_ref() } + } +} + +impl AsMut for BoxedShard { + fn as_mut(&mut self) -> &mut UnwindEntryShard { + unsafe { self.0.as_mut() } + } +} diff --git a/agent/src/flow_generator/protocol_logs/tls.rs b/agent/src/flow_generator/protocol_logs/tls.rs index 30888a697e4..222cf5b630a 100644 --- a/agent/src/flow_generator/protocol_logs/tls.rs +++ b/agent/src/flow_generator/protocol_logs/tls.rs @@ -16,7 +16,7 @@ use std::fmt::Display; -use chrono::{prelude::NaiveDateTime, Utc}; +use chrono::{DateTime, Utc}; use serde::Serialize; use super::pb_adapter::{ @@ -370,12 +370,9 @@ impl From for L7ProtocolSendLog { if !f.client_cert_not_before.is_zero() { attributes.push(KeyVal { key: "client_cert_not_before".to_string(), - val: NaiveDateTime::from_timestamp_opt( - f.client_cert_not_before.as_secs() as i64, - 0, - ) - .unwrap() - .to_string(), + val: DateTime::from_timestamp(f.client_cert_not_before.as_secs() as i64, 0) + .unwrap() + .to_string(), }); } if !f.client_cert_not_after.is_zero() { @@ -383,7 +380,7 @@ impl From for L7ProtocolSendLog { (f.client_cert_not_after.as_secs() as i64 - now) as f32 / Self::SECONDS_PER_DAY; attributes.push(KeyVal { key: "client_cert_not_after".to_string(), - val: NaiveDateTime::from_timestamp_opt(f.client_cert_not_after.as_secs() as i64, 0) + val: DateTime::from_timestamp(f.client_cert_not_after.as_secs() as i64, 0) .unwrap() .to_string(), }); @@ -395,12 +392,9 @@ impl From for L7ProtocolSendLog { if !f.server_cert_not_before.is_zero() { attributes.push(KeyVal { key: "server_cert_not_before".to_string(), - val: NaiveDateTime::from_timestamp_opt( - f.server_cert_not_before.as_secs() as i64, - 0, - ) - .unwrap() - .to_string(), + val: DateTime::from_timestamp(f.server_cert_not_before.as_secs() as i64, 0) + .unwrap() + .to_string(), }); } if !f.server_cert_not_after.is_zero() { @@ -408,7 +402,7 @@ impl From for L7ProtocolSendLog { (f.server_cert_not_after.as_secs() as i64 - now) as f32 / Self::SECONDS_PER_DAY; attributes.push(KeyVal { key: "server_cert_not_after".to_string(), - val: NaiveDateTime::from_timestamp_opt(f.server_cert_not_after.as_secs() as i64, 0) + val: DateTime::from_timestamp(f.server_cert_not_after.as_secs() as i64, 0) .unwrap() .to_string(), });