From 3d2af1f1c6f61109cc8109fc0a4af4f8be79634b Mon Sep 17 00:00:00 2001 From: Yoh Deadfall Date: Mon, 30 Sep 2024 12:51:10 +0300 Subject: [PATCH 1/2] Always panic if len of verlena exceeds the maximum --- pgrx/src/datum/into.rs | 6 +++--- pgrx/src/varlena.rs | 2 +- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/pgrx/src/datum/into.rs b/pgrx/src/datum/into.rs index 8697698c62..a1ec5060c6 100644 --- a/pgrx/src/datum/into.rs +++ b/pgrx/src/datum/into.rs @@ -337,7 +337,7 @@ impl_into_datum_c_str!(&CStr); impl<'a> IntoDatum for &'a [u8] { /// # Panics /// - /// This function will panic if the string being converted to a datum is longer than a `u32`. + /// This function will panic if the string being converted to a datum is longer than a half of [`i32::MAX`]. #[inline] fn into_datum(self) -> Option { let len = pg_sys::VARHDRSZ + self.len(); @@ -352,9 +352,9 @@ impl<'a> IntoDatum for &'a [u8] { // This is the same as Postgres' `#define SET_VARSIZE_4B` (which have over in // `pgrx/src/varlena.rs`), however we're asserting that the input string isn't too big - // for a Postgres varlena, since it's limited to 32bits -- in reality it's about half + // for a Postgres varlena, since it's limited to 32bits -- in reality it's a quarter // that length, but this is good enough - debug_assert!(len < (u32::MAX as usize >> 2)); + assert!(len < (u32::MAX as usize >> 2)); set_varsize_4b(varlena, len as i32); // SAFETY: src and dest pointers are valid, exactly `self.len()` bytes long, diff --git a/pgrx/src/varlena.rs b/pgrx/src/varlena.rs index 3aa6d1f8ea..fb5c78a7bd 100644 --- a/pgrx/src/varlena.rs +++ b/pgrx/src/varlena.rs @@ -15,7 +15,7 @@ use core::{ops::DerefMut, slice, str}; /// # Safety /// /// The caller asserts the specified `ptr` really is a non-null, palloc'd [`pg_sys::varlena`] pointer -/// that is aligned to 4 bytes. +/// that is aligned to 4 bytes, and that the `len` is a half of [`i32::MAX`] #[inline(always)] pub unsafe fn set_varsize_4b(ptr: *mut pg_sys::varlena, len: i32) { // #define SET_VARSIZE_4B(PTR,len) \ From 20443a84118f3c9398b5429346348dffb177958c Mon Sep 17 00:00:00 2001 From: Yoh Deadfall Date: Wed, 2 Oct 2024 16:08:23 +0300 Subject: [PATCH 2/2] Moved assertion before alloction --- pgrx/src/datum/into.rs | 13 +++++++------ 1 file changed, 7 insertions(+), 6 deletions(-) diff --git a/pgrx/src/datum/into.rs b/pgrx/src/datum/into.rs index a1ec5060c6..50f69111df 100644 --- a/pgrx/src/datum/into.rs +++ b/pgrx/src/datum/into.rs @@ -337,10 +337,12 @@ impl_into_datum_c_str!(&CStr); impl<'a> IntoDatum for &'a [u8] { /// # Panics /// - /// This function will panic if the string being converted to a datum is longer than a half of [`i32::MAX`]. + /// This function will panic if the string being converted to a datum + // is longer than 1 GiB including 4 bytes used for a header. #[inline] fn into_datum(self) -> Option { - let len = pg_sys::VARHDRSZ + self.len(); + let len = self.len().saturating_add(pg_sys::VARHDRSZ); + assert!(len < (u32::MAX as usize >> 2)); unsafe { // SAFETY: palloc gives us a valid pointer and if there's not enough memory it'll raise an error let varlena = pg_sys::palloc(len) as *mut pg_sys::varlena; @@ -351,10 +353,9 @@ impl<'a> IntoDatum for &'a [u8] { &mut varlena.cast::().as_mut().unwrap_unchecked().va_4byte; // This is the same as Postgres' `#define SET_VARSIZE_4B` (which have over in - // `pgrx/src/varlena.rs`), however we're asserting that the input string isn't too big - // for a Postgres varlena, since it's limited to 32bits -- in reality it's a quarter - // that length, but this is good enough - assert!(len < (u32::MAX as usize >> 2)); + // `pgrx/src/varlena.rs`), however we're asserting above that the input string + // isn't too big for a Postgres varlena, since it's limited to 32 bits and, + // in reality, it's a quarter that length, but this is good enough set_varsize_4b(varlena, len as i32); // SAFETY: src and dest pointers are valid, exactly `self.len()` bytes long,