Skip to content

Commit

Permalink
Added a utility function to ensure we never have an issue with 0-leng…
Browse files Browse the repository at this point in the history
…th slices from pointers again

Added a clippy lint to ensure we use it.
  • Loading branch information
alex committed Jul 21, 2024
1 parent ad70a0b commit d605d24
Show file tree
Hide file tree
Showing 8 changed files with 66 additions and 40 deletions.
4 changes: 4 additions & 0 deletions clippy.toml
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
disallowed-methods = [
{ path = "std::slice::from_raw_parts", reason = "use util::from_raw_parts instead" },
{ path = "std::slice::from_raw_parts_mut", reason = "use util::from_raw_parts_mut instead" },
]
11 changes: 5 additions & 6 deletions openssl/src/asn1.rs
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,6 @@ use std::convert::TryInto;
use std::ffi::CString;
use std::fmt;
use std::ptr;
use std::slice;
use std::str;

use crate::bio::MemBio;
Expand All @@ -41,7 +40,7 @@ use crate::error::ErrorStack;
use crate::nid::Nid;
use crate::stack::Stackable;
use crate::string::OpensslString;
use crate::{cvt, cvt_p};
use crate::{cvt, cvt_p, util};
use openssl_macros::corresponds;

foreign_type_and_impl_send_sync! {
Expand Down Expand Up @@ -457,7 +456,7 @@ impl Asn1StringRef {
/// [`as_utf8`]: struct.Asn1String.html#method.as_utf8
#[corresponds(ASN1_STRING_get0_data)]
pub fn as_slice(&self) -> &[u8] {
unsafe { slice::from_raw_parts(ASN1_STRING_get0_data(self.as_ptr()), self.len()) }
unsafe { util::from_raw_parts(ASN1_STRING_get0_data(self.as_ptr()), self.len()) }
}

/// Returns the number of bytes in the string.
Expand Down Expand Up @@ -597,7 +596,7 @@ impl Asn1BitStringRef {
/// Returns the Asn1BitString as a slice.
#[corresponds(ASN1_STRING_get0_data)]
pub fn as_slice(&self) -> &[u8] {
unsafe { slice::from_raw_parts(ASN1_STRING_get0_data(self.as_ptr() as *mut _), self.len()) }
unsafe { util::from_raw_parts(ASN1_STRING_get0_data(self.as_ptr() as *mut _), self.len()) }
}

/// Returns the number of bytes in the string.
Expand Down Expand Up @@ -637,7 +636,7 @@ impl Asn1OctetStringRef {
/// Returns the octet string as an array of bytes.
#[corresponds(ASN1_STRING_get0_data)]
pub fn as_slice(&self) -> &[u8] {
unsafe { slice::from_raw_parts(ASN1_STRING_get0_data(self.as_ptr().cast()), self.len()) }
unsafe { util::from_raw_parts(ASN1_STRING_get0_data(self.as_ptr().cast()), self.len()) }
}

/// Returns the number of bytes in the octet string.
Expand Down Expand Up @@ -701,7 +700,7 @@ impl Asn1Object {
pub fn as_slice(&self) -> &[u8] {
unsafe {
let len = ffi::OBJ_length(self.as_ptr());
slice::from_raw_parts(ffi::OBJ_get0_data(self.as_ptr()), len)
util::from_raw_parts(ffi::OBJ_get0_data(self.as_ptr()), len)
}
}
}
Expand Down
4 changes: 2 additions & 2 deletions openssl/src/bio.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,10 +2,10 @@ use cfg_if::cfg_if;
use libc::c_int;
use std::marker::PhantomData;
use std::ptr;
use std::slice;

use crate::cvt_p;
use crate::error::ErrorStack;
use crate::util;

pub struct MemBioSlice<'a>(*mut ffi::BIO, PhantomData<&'a [u8]>);

Expand Down Expand Up @@ -66,7 +66,7 @@ impl MemBio {
if len == 0 {
&[]
} else {
slice::from_raw_parts(ptr as *const _ as *const _, len as usize)
util::from_raw_parts(ptr as *const _ as *const _, len as usize)
}
}
}
Expand Down
7 changes: 3 additions & 4 deletions openssl/src/ssl/bio.rs
Original file line number Diff line number Diff line change
Expand Up @@ -9,10 +9,9 @@ use std::io;
use std::io::prelude::*;
use std::panic::{catch_unwind, AssertUnwindSafe};
use std::ptr;
use std::slice;

use crate::cvt_p;
use crate::error::ErrorStack;
use crate::{cvt_p, util};

pub struct StreamState<S> {
pub stream: S,
Expand Down Expand Up @@ -89,7 +88,7 @@ unsafe extern "C" fn bwrite<S: Write>(bio: *mut BIO, buf: *const c_char, len: c_
BIO_clear_retry_flags(bio);

let state = state::<S>(bio);
let buf = slice::from_raw_parts(buf as *const _, len as usize);
let buf = util::from_raw_parts(buf as *const _, len as usize);

match catch_unwind(AssertUnwindSafe(|| state.stream.write(buf))) {
Ok(Ok(len)) => len as c_int,
Expand All @@ -111,7 +110,7 @@ unsafe extern "C" fn bread<S: Read>(bio: *mut BIO, buf: *mut c_char, len: c_int)
BIO_clear_retry_flags(bio);

let state = state::<S>(bio);
let buf = slice::from_raw_parts_mut(buf as *mut _, len as usize);
let buf = util::from_raw_parts_mut(buf as *mut _, len as usize);

match catch_unwind(AssertUnwindSafe(|| state.stream.read(buf))) {
Ok(Ok(len)) => len as c_int,
Expand Down
22 changes: 11 additions & 11 deletions openssl/src/ssl/callbacks.rs
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,6 @@ use libc::{c_int, c_uchar, c_uint, c_void};
use std::ffi::CStr;
use std::mem;
use std::ptr;
use std::slice;
#[cfg(any(ossl111, boringssl))]
use std::str;
use std::sync::Arc;
Expand All @@ -28,6 +27,7 @@ use crate::ssl::{
};
#[cfg(ossl111)]
use crate::ssl::{ClientHelloResponse, ExtensionContext};
use crate::util;
#[cfg(any(ossl111, boringssl))]
use crate::util::ForeignTypeRefExt;
#[cfg(ossl111)]
Expand Down Expand Up @@ -85,9 +85,9 @@ where
None
};
// Give the callback mutable slices into which it can write the identity and psk.
let identity_sl = slice::from_raw_parts_mut(identity as *mut u8, max_identity_len as usize);
let identity_sl = util::from_raw_parts_mut(identity as *mut u8, max_identity_len as usize);
#[allow(clippy::unnecessary_cast)]
let psk_sl = slice::from_raw_parts_mut(psk as *mut u8, max_psk_len as usize);
let psk_sl = util::from_raw_parts_mut(psk as *mut u8, max_psk_len as usize);
match (*callback)(ssl, hint, identity_sl, psk_sl) {
Ok(psk_len) => psk_len as u32,
Err(e) => {
Expand Down Expand Up @@ -126,7 +126,7 @@ where
};
// Give the callback mutable slices into which it can write the psk.
#[allow(clippy::unnecessary_cast)]
let psk_sl = slice::from_raw_parts_mut(psk as *mut u8, max_psk_len as usize);
let psk_sl = util::from_raw_parts_mut(psk as *mut u8, max_psk_len as usize);
match (*callback)(ssl, identity, psk_sl) {
Ok(psk_len) => psk_len as u32,
Err(e) => {
Expand Down Expand Up @@ -197,7 +197,7 @@ where
.ex_data(SslContext::cached_ex_index::<F>())
.expect("BUG: alpn callback missing") as *const F;
#[allow(clippy::unnecessary_cast)]
let protos = slice::from_raw_parts(inbuf as *const u8, inlen as usize);
let protos = util::from_raw_parts(inbuf as *const u8, inlen as usize);

match (*callback)(ssl, protos) {
Ok(proto) => {
Expand Down Expand Up @@ -416,7 +416,7 @@ where
.ex_data(SslContext::cached_ex_index::<F>())
.expect("BUG: get session callback missing") as *const F;
#[allow(clippy::unnecessary_cast)]
let data = slice::from_raw_parts(data as *const u8, len as usize);
let data = util::from_raw_parts(data as *const u8, len as usize);

match (*callback)(ssl, data) {
Some(session) => {
Expand Down Expand Up @@ -460,7 +460,7 @@ where
.ex_data(SslContext::cached_ex_index::<F>())
.expect("BUG: stateless cookie generate callback missing") as *const F;
#[allow(clippy::unnecessary_cast)]
let slice = slice::from_raw_parts_mut(cookie as *mut u8, ffi::SSL_COOKIE_LENGTH as usize);
let slice = util::from_raw_parts_mut(cookie as *mut u8, ffi::SSL_COOKIE_LENGTH as usize);
match (*callback)(ssl, slice) {
Ok(len) => {
*cookie_len = len as size_t;
Expand Down Expand Up @@ -488,7 +488,7 @@ where
.ex_data(SslContext::cached_ex_index::<F>())
.expect("BUG: stateless cookie verify callback missing") as *const F;
#[allow(clippy::unnecessary_cast)]
let slice = slice::from_raw_parts(cookie as *const c_uchar as *const u8, cookie_len);
let slice = util::from_raw_parts(cookie as *const c_uchar as *const u8, cookie_len);
(*callback)(ssl, slice) as c_int
}

Expand All @@ -511,7 +511,7 @@ where
// compatibility. See comments in dtls1.h.
#[allow(clippy::unnecessary_cast)]
let slice =
slice::from_raw_parts_mut(cookie as *mut u8, ffi::DTLS1_COOKIE_LENGTH as usize - 1);
util::from_raw_parts_mut(cookie as *mut u8, ffi::DTLS1_COOKIE_LENGTH as usize - 1);
match (*callback)(ssl, slice) {
Ok(len) => {
*cookie_len = len as c_uint;
Expand Down Expand Up @@ -551,7 +551,7 @@ where
.expect("BUG: cookie verify callback missing") as *const F;
#[allow(clippy::unnecessary_cast)]
let slice =
slice::from_raw_parts(cookie as *const c_uchar as *const u8, cookie_len as usize);
util::from_raw_parts(cookie as *const c_uchar as *const u8, cookie_len as usize);
(*callback)(ssl, slice) as c_int
}
}
Expand Down Expand Up @@ -663,7 +663,7 @@ where
.expect("BUG: custom ext parse callback missing") as *const F;
let ectx = ExtensionContext::from_bits_truncate(context);
#[allow(clippy::unnecessary_cast)]
let slice = slice::from_raw_parts(input as *const u8, inlen);
let slice = util::from_raw_parts(input as *const u8, inlen);
let cert = if ectx.contains(ExtensionContext::TLS1_3_CERTIFICATE) {
Some((chainidx, X509Ref::from_ptr(x)))
} else {
Expand Down
22 changes: 11 additions & 11 deletions openssl/src/ssl/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -77,6 +77,7 @@ use crate::ssl::bio::BioMethod;
use crate::ssl::callbacks::*;
use crate::ssl::error::InnerError;
use crate::stack::{Stack, StackRef, Stackable};
use crate::util;
use crate::util::{ForeignTypeExt, ForeignTypeRefExt};
use crate::x509::store::{X509Store, X509StoreBuilderRef, X509StoreRef};
#[cfg(any(ossl102, boringssl, libressl261))]
Expand All @@ -101,7 +102,6 @@ use std::ops::{Deref, DerefMut};
use std::panic::resume_unwind;
use std::path::Path;
use std::ptr;
use std::slice;
use std::str;
use std::sync::{Arc, Mutex};

Expand Down Expand Up @@ -708,7 +708,7 @@ pub fn select_next_proto<'a>(server: &[u8], client: &'a [u8]) -> Option<&'a [u8]
client.len() as c_uint,
);
if r == ffi::OPENSSL_NPN_NEGOTIATED {
Some(slice::from_raw_parts(out as *const u8, outlen as usize))
Some(util::from_raw_parts(out as *const u8, outlen as usize))
} else {
None
}
Expand Down Expand Up @@ -2174,7 +2174,7 @@ impl SslSessionRef {
let mut len = 0;
let p = ffi::SSL_SESSION_get_id(self.as_ptr(), &mut len);
#[allow(clippy::unnecessary_cast)]
slice::from_raw_parts(p as *const u8, len as usize)
util::from_raw_parts(p as *const u8, len as usize)
}
}

Expand Down Expand Up @@ -2650,7 +2650,7 @@ impl SslRef {
if data.is_null() {
None
} else {
Some(slice::from_raw_parts(data, len as usize))
Some(util::from_raw_parts(data, len as usize))
}
}
}
Expand Down Expand Up @@ -2928,7 +2928,7 @@ impl SslRef {
if len < 0 {
None
} else {
Some(slice::from_raw_parts(p as *const u8, len as usize))
Some(util::from_raw_parts(p as *const u8, len as usize))
}
}
}
Expand Down Expand Up @@ -3099,7 +3099,7 @@ impl SslRef {
if len == 0 {
None
} else {
Some(slice::from_raw_parts(ptr, len))
Some(util::from_raw_parts(ptr, len))
}
}
}
Expand All @@ -3118,7 +3118,7 @@ impl SslRef {
if len == 0 {
None
} else {
Some(slice::from_raw_parts(ptr, len))
Some(util::from_raw_parts(ptr, len))
}
}
}
Expand All @@ -3137,7 +3137,7 @@ impl SslRef {
if len == 0 {
None
} else {
Some(slice::from_raw_parts(ptr, len))
Some(util::from_raw_parts(ptr, len))
}
}
}
Expand Down Expand Up @@ -3191,7 +3191,7 @@ impl SslRef {
if len == 0 {
None
} else {
Some(slice::from_raw_parts(ptr, len))
Some(util::from_raw_parts(ptr, len))
}
}
}
Expand Down Expand Up @@ -3764,7 +3764,7 @@ impl<S: Read + Write> SslStream<S> {
pub fn ssl_read(&mut self, buf: &mut [u8]) -> Result<usize, Error> {
// SAFETY: `ssl_read_uninit` does not de-initialize the buffer.
unsafe {
self.ssl_read_uninit(slice::from_raw_parts_mut(
self.ssl_read_uninit(util::from_raw_parts_mut(
buf.as_mut_ptr().cast::<MaybeUninit<u8>>(),
buf.len(),
))
Expand Down Expand Up @@ -3997,7 +3997,7 @@ impl<S: Read + Write> Read for SslStream<S> {
fn read(&mut self, buf: &mut [u8]) -> io::Result<usize> {
// SAFETY: `read_uninit` does not de-initialize the buffer
unsafe {
self.read_uninit(slice::from_raw_parts_mut(
self.read_uninit(util::from_raw_parts_mut(
buf.as_mut_ptr().cast::<MaybeUninit<u8>>(),
buf.len(),
))
Expand Down
27 changes: 26 additions & 1 deletion openssl/src/util.rs
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
use crate::util;
use crate::error::ErrorStack;
use foreign_types::{ForeignType, ForeignTypeRef};
use libc::{c_char, c_int, c_void};
Expand Down Expand Up @@ -49,7 +50,7 @@ where
let callback = &mut *(cb_state as *mut CallbackState<F>);

let result = panic::catch_unwind(AssertUnwindSafe(|| {
let pass_slice = slice::from_raw_parts_mut(buf as *mut u8, size as usize);
let pass_slice = util::from_raw_parts_mut(buf as *mut u8, size as usize);
callback.cb.take().unwrap()(pass_slice)
}));

Expand Down Expand Up @@ -91,3 +92,27 @@ pub trait ForeignTypeRefExt: ForeignTypeRef {
}
}
impl<FT: ForeignTypeRef> ForeignTypeRefExt for FT {}

/// The same as `slice::from_raw_parts`, except that `data` may be `NULL` if
/// `len` is 0.
pub unsafe fn from_raw_parts<'a, T>(data: *const T, len: usize) -> &'a [T] {
if len == 0 {
&[]
} else {
// Using this to implement the preferred API
#[allow(clippy::disallowed_methods)]
slice::from_raw_parts(data, len)
}
}

/// The same as `slice::from_raw_parts_mut`, except that `data` may be `NULL`
/// if `len` is 0.
pub unsafe fn from_raw_parts_mut<'a, T>(data: *mut T, len: usize) -> &'a mut [T] {
if len == 0 {
&mut []
} else {
// Using this to implement the preferred API
#[allow(clippy::disallowed_methods)]
slice::from_raw_parts_mut(data, len)
}
}
Loading

0 comments on commit d605d24

Please sign in to comment.