Skip to content

Commit

Permalink
Fix unsoundness via impure AsRef (rust-rocksdb#705)
Browse files Browse the repository at this point in the history
* Fix unsoundness via impure AsRef

---------

Co-authored-by: Oleksandr Anyshchenko <[email protected]>
Co-authored-by: Zaidoon Abd Al Hadi <[email protected]>
  • Loading branch information
3 people authored Sep 20, 2024
1 parent 5935b8f commit b8bf373
Show file tree
Hide file tree
Showing 4 changed files with 128 additions and 56 deletions.
10 changes: 8 additions & 2 deletions src/db.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1104,7 +1104,10 @@ impl<T: ThreadMode, D: DBInner> DBCommon<T, D> {
{
let (keys, keys_sizes): (Vec<Box<[u8]>>, Vec<_>) = keys
.into_iter()
.map(|k| (Box::from(k.as_ref()), k.as_ref().len()))
.map(|k| {
let k = k.as_ref();
(Box::from(k), k.len())
})
.unzip();
let ptr_keys: Vec<_> = keys.iter().map(|k| k.as_ptr() as *const c_char).collect();

Expand Down Expand Up @@ -1153,7 +1156,10 @@ impl<T: ThreadMode, D: DBInner> DBCommon<T, D> {
{
let (cfs_and_keys, keys_sizes): (Vec<(_, Box<[u8]>)>, Vec<_>) = keys
.into_iter()
.map(|(cf, key)| ((cf, Box::from(key.as_ref())), key.as_ref().len()))
.map(|(cf, key)| {
let key = key.as_ref();
((cf, Box::from(key)), key.len())
})
.unzip();
let ptr_keys: Vec<_> = cfs_and_keys
.iter()
Expand Down
80 changes: 50 additions & 30 deletions src/transactions/transaction.rs
Original file line number Diff line number Diff line change
Expand Up @@ -321,12 +321,13 @@ impl<'db, DB> Transaction<'db, DB> {
key: K,
readopts: &ReadOptions,
) -> Result<Option<DBPinnableSlice>, Error> {
let key = key.as_ref();
unsafe {
let val = ffi_try!(ffi::rocksdb_transaction_get_pinned(
self.inner,
readopts.inner,
key.as_ref().as_ptr() as *const c_char,
key.as_ref().len(),
key.as_ptr() as *const c_char,
key.len(),
));
if val.is_null() {
Ok(None)
Expand Down Expand Up @@ -359,13 +360,14 @@ impl<'db, DB> Transaction<'db, DB> {
key: K,
readopts: &ReadOptions,
) -> Result<Option<DBPinnableSlice>, Error> {
let key = key.as_ref();
unsafe {
let val = ffi_try!(ffi::rocksdb_transaction_get_pinned_cf(
self.inner,
readopts.inner,
cf.inner(),
key.as_ref().as_ptr() as *const c_char,
key.as_ref().len(),
key.as_ptr() as *const c_char,
key.len(),
));
if val.is_null() {
Ok(None)
Expand Down Expand Up @@ -399,12 +401,13 @@ impl<'db, DB> Transaction<'db, DB> {
exclusive: bool,
opts: &ReadOptions,
) -> Result<Option<DBPinnableSlice>, Error> {
let key = key.as_ref();
unsafe {
let val = ffi_try!(ffi::rocksdb_transaction_get_pinned_for_update(
self.inner,
opts.inner,
key.as_ref().as_ptr() as *const c_char,
key.as_ref().len() as size_t,
key.as_ptr() as *const c_char,
key.len() as size_t,
u8::from(exclusive),
));
if val.is_null() {
Expand Down Expand Up @@ -461,13 +464,14 @@ impl<'db, DB> Transaction<'db, DB> {
exclusive: bool,
opts: &ReadOptions,
) -> Result<Option<DBPinnableSlice>, Error> {
let key = key.as_ref();
unsafe {
let val = ffi_try!(ffi::rocksdb_transaction_get_pinned_for_update_cf(
self.inner,
opts.inner,
cf.inner(),
key.as_ref().as_ptr() as *const c_char,
key.as_ref().len() as size_t,
key.as_ptr() as *const c_char,
key.len() as size_t,
u8::from(exclusive),
));
if val.is_null() {
Expand Down Expand Up @@ -499,7 +503,10 @@ impl<'db, DB> Transaction<'db, DB> {
{
let (keys, keys_sizes): (Vec<Box<[u8]>>, Vec<_>) = keys
.into_iter()
.map(|k| (Box::from(k.as_ref()), k.as_ref().len()))
.map(|key| {
let key = key.as_ref();
(Box::from(key), key.len())
})
.unzip();
let ptr_keys: Vec<_> = keys.iter().map(|k| k.as_ptr() as *const c_char).collect();

Expand Down Expand Up @@ -548,7 +555,10 @@ impl<'db, DB> Transaction<'db, DB> {
{
let (cfs_and_keys, keys_sizes): (Vec<(_, Box<[u8]>)>, Vec<_>) = keys
.into_iter()
.map(|(cf, key)| ((cf, Box::from(key.as_ref())), key.as_ref().len()))
.map(|(cf, key)| {
let key = key.as_ref();
((cf, Box::from(key)), key.len())
})
.unzip();
let ptr_keys: Vec<_> = cfs_and_keys
.iter()
Expand Down Expand Up @@ -585,13 +595,15 @@ impl<'db, DB> Transaction<'db, DB> {
///
/// [`put_cf`]: Self::put_cf
pub fn put<K: AsRef<[u8]>, V: AsRef<[u8]>>(&self, key: K, value: V) -> Result<(), Error> {
let key = key.as_ref();
let value = value.as_ref();
unsafe {
ffi_try!(ffi::rocksdb_transaction_put(
self.inner,
key.as_ref().as_ptr() as *const c_char,
key.as_ref().len() as size_t,
value.as_ref().as_ptr() as *const c_char,
value.as_ref().len() as size_t,
key.as_ptr() as *const c_char,
key.len() as size_t,
value.as_ptr() as *const c_char,
value.len() as size_t,
));
Ok(())
}
Expand All @@ -618,14 +630,16 @@ impl<'db, DB> Transaction<'db, DB> {
key: K,
value: V,
) -> Result<(), Error> {
let key = key.as_ref();
let value = value.as_ref();
unsafe {
ffi_try!(ffi::rocksdb_transaction_put_cf(
self.inner,
cf.inner(),
key.as_ref().as_ptr() as *const c_char,
key.as_ref().len() as size_t,
value.as_ref().as_ptr() as *const c_char,
value.as_ref().len() as size_t,
key.as_ptr() as *const c_char,
key.len() as size_t,
value.as_ptr() as *const c_char,
value.len() as size_t,
));
Ok(())
}
Expand All @@ -637,13 +651,15 @@ impl<'db, DB> Transaction<'db, DB> {
///
/// [`merge_cf`]: Self::merge_cf
pub fn merge<K: AsRef<[u8]>, V: AsRef<[u8]>>(&self, key: K, value: V) -> Result<(), Error> {
let key = key.as_ref();
let value = value.as_ref();
unsafe {
ffi_try!(ffi::rocksdb_transaction_merge(
self.inner,
key.as_ref().as_ptr() as *const c_char,
key.as_ref().len() as size_t,
value.as_ref().as_ptr() as *const c_char,
value.as_ref().len() as size_t
key.as_ptr() as *const c_char,
key.len() as size_t,
value.as_ptr() as *const c_char,
value.len() as size_t
));
Ok(())
}
Expand All @@ -670,14 +686,16 @@ impl<'db, DB> Transaction<'db, DB> {
key: K,
value: V,
) -> Result<(), Error> {
let key = key.as_ref();
let value = value.as_ref();
unsafe {
ffi_try!(ffi::rocksdb_transaction_merge_cf(
self.inner,
cf.inner(),
key.as_ref().as_ptr() as *const c_char,
key.as_ref().len() as size_t,
value.as_ref().as_ptr() as *const c_char,
value.as_ref().len() as size_t
key.as_ptr() as *const c_char,
key.len() as size_t,
value.as_ptr() as *const c_char,
value.len() as size_t
));
Ok(())
}
Expand All @@ -689,11 +707,12 @@ impl<'db, DB> Transaction<'db, DB> {
///
/// [`delete_cf`]: Self::delete_cf
pub fn delete<K: AsRef<[u8]>>(&self, key: K) -> Result<(), Error> {
let key = key.as_ref();
unsafe {
ffi_try!(ffi::rocksdb_transaction_delete(
self.inner,
key.as_ref().as_ptr() as *const c_char,
key.as_ref().len() as size_t
key.as_ptr() as *const c_char,
key.len() as size_t
));
}
Ok(())
Expand All @@ -718,12 +737,13 @@ impl<'db, DB> Transaction<'db, DB> {
cf: &impl AsColumnFamilyRef,
key: K,
) -> Result<(), Error> {
let key = key.as_ref();
unsafe {
ffi_try!(ffi::rocksdb_transaction_delete_cf(
self.inner,
cf.inner(),
key.as_ref().as_ptr() as *const c_char,
key.as_ref().len() as size_t
key.as_ptr() as *const c_char,
key.len() as size_t
));
}
Ok(())
Expand Down
65 changes: 41 additions & 24 deletions src/transactions/transaction_db.rs
Original file line number Diff line number Diff line change
Expand Up @@ -509,13 +509,14 @@ impl<T: ThreadMode> TransactionDB<T> {
key: K,
readopts: &ReadOptions,
) -> Result<Option<DBPinnableSlice>, Error> {
let key = key.as_ref();
unsafe {
let val = ffi_try!(ffi::rocksdb_transactiondb_get_pinned_cf(
self.inner,
readopts.inner,
cf.inner(),
key.as_ref().as_ptr() as *const c_char,
key.as_ref().len() as size_t,
key.as_ptr() as *const c_char,
key.len() as size_t,
));
if val.is_null() {
Ok(None)
Expand Down Expand Up @@ -546,7 +547,10 @@ impl<T: ThreadMode> TransactionDB<T> {
{
let (keys, keys_sizes): (Vec<Box<[u8]>>, Vec<_>) = keys
.into_iter()
.map(|k| (Box::from(k.as_ref()), k.as_ref().len()))
.map(|key| {
let key = key.as_ref();
(Box::from(key), key.len())
})
.unzip();
let ptr_keys: Vec<_> = keys.iter().map(|k| k.as_ptr() as *const c_char).collect();

Expand Down Expand Up @@ -595,7 +599,10 @@ impl<T: ThreadMode> TransactionDB<T> {
{
let (cfs_and_keys, keys_sizes): (Vec<(_, Box<[u8]>)>, Vec<_>) = keys
.into_iter()
.map(|(cf, key)| ((cf, Box::from(key.as_ref())), key.as_ref().len()))
.map(|(cf, key)| {
let key = key.as_ref();
((cf, Box::from(key)), key.len())
})
.unzip();
let ptr_keys: Vec<_> = cfs_and_keys
.iter()
Expand Down Expand Up @@ -647,14 +654,16 @@ impl<T: ThreadMode> TransactionDB<T> {
K: AsRef<[u8]>,
V: AsRef<[u8]>,
{
let key = key.as_ref();
let value = value.as_ref();
unsafe {
ffi_try!(ffi::rocksdb_transactiondb_put(
self.inner,
writeopts.inner,
key.as_ref().as_ptr() as *const c_char,
key.as_ref().len() as size_t,
value.as_ref().as_ptr() as *const c_char,
value.as_ref().len() as size_t
key.as_ptr() as *const c_char,
key.len() as size_t,
value.as_ptr() as *const c_char,
value.len() as size_t
));
}
Ok(())
Expand All @@ -671,15 +680,17 @@ impl<T: ThreadMode> TransactionDB<T> {
K: AsRef<[u8]>,
V: AsRef<[u8]>,
{
let key = key.as_ref();
let value = value.as_ref();
unsafe {
ffi_try!(ffi::rocksdb_transactiondb_put_cf(
self.inner,
writeopts.inner,
cf.inner(),
key.as_ref().as_ptr() as *const c_char,
key.as_ref().len() as size_t,
value.as_ref().as_ptr() as *const c_char,
value.as_ref().len() as size_t
key.as_ptr() as *const c_char,
key.len() as size_t,
value.as_ptr() as *const c_char,
value.len() as size_t
));
}
Ok(())
Expand Down Expand Up @@ -725,14 +736,16 @@ impl<T: ThreadMode> TransactionDB<T> {
K: AsRef<[u8]>,
V: AsRef<[u8]>,
{
let key = key.as_ref();
let value = value.as_ref();
unsafe {
ffi_try!(ffi::rocksdb_transactiondb_merge(
self.inner,
writeopts.inner,
key.as_ref().as_ptr() as *const c_char,
key.as_ref().len() as size_t,
value.as_ref().as_ptr() as *const c_char,
value.as_ref().len() as size_t,
key.as_ptr() as *const c_char,
key.len() as size_t,
value.as_ptr() as *const c_char,
value.len() as size_t,
));
Ok(())
}
Expand All @@ -749,15 +762,17 @@ impl<T: ThreadMode> TransactionDB<T> {
K: AsRef<[u8]>,
V: AsRef<[u8]>,
{
let key = key.as_ref();
let value = value.as_ref();
unsafe {
ffi_try!(ffi::rocksdb_transactiondb_merge_cf(
self.inner,
writeopts.inner,
cf.inner(),
key.as_ref().as_ptr() as *const c_char,
key.as_ref().len() as size_t,
value.as_ref().as_ptr() as *const c_char,
value.as_ref().len() as size_t,
key.as_ptr() as *const c_char,
key.len() as size_t,
value.as_ptr() as *const c_char,
value.len() as size_t,
));
Ok(())
}
Expand All @@ -780,12 +795,13 @@ impl<T: ThreadMode> TransactionDB<T> {
key: K,
writeopts: &WriteOptions,
) -> Result<(), Error> {
let key = key.as_ref();
unsafe {
ffi_try!(ffi::rocksdb_transactiondb_delete(
self.inner,
writeopts.inner,
key.as_ref().as_ptr() as *const c_char,
key.as_ref().len() as size_t,
key.as_ptr() as *const c_char,
key.len() as size_t,
));
}
Ok(())
Expand All @@ -797,13 +813,14 @@ impl<T: ThreadMode> TransactionDB<T> {
key: K,
writeopts: &WriteOptions,
) -> Result<(), Error> {
let key = key.as_ref();
unsafe {
ffi_try!(ffi::rocksdb_transactiondb_delete_cf(
self.inner,
writeopts.inner,
cf.inner(),
key.as_ref().as_ptr() as *const c_char,
key.as_ref().len() as size_t,
key.as_ptr() as *const c_char,
key.len() as size_t,
));
}
Ok(())
Expand Down
Loading

0 comments on commit b8bf373

Please sign in to comment.