0.5.0 (current) Thoroughness: Medium Understanding: Medium
by gitlab.com/phgsng on 2019-10-04
This review is from Crev, a distributed system for code reviews. To add your review, set up cargo-crev
.
0.5.0 (current) Thoroughness: Medium Understanding: Medium
by gitlab.com/phgsng on 2019-10-04
This review is from cargo-vet. To add your review, set up cargo-vet
and submit your URL to its registry.
0.5.0 (current)
From kornelski/crev-proofs copy of git.savannah.gnu.org.
Packaged for Guix (crates-io)
cargo-vet does not verify reviewers' identity. You have to fully trust the source the audits are from.
May have been packaged automatically without a review
Crates in the crates.io registry are tarball snapshots uploaded by crates' publishers. The registry is not using crates' git repositories. There is absolutely no guarantee that the repository URL declared by the crate belongs to the crate, or that the code in the repository is the code inside the published tarball.
To review the actual code of the crate, it's best to use cargo crev open unix_socket
. Alternatively, you can download the tarball of unix_socket v0.5.0 or view the source online.
Disclaimer: as far as syscall usage is concerned, this review considers only the behavior on Linux.
Cons
shutdown()
twice on one socket. The result is handled safely though even under programmer errors.try!()
and thus may not build with a future version of Rust.Pros
unsafe
is never gratuitous.lib.rs
general considerations
io::Result
.unsafe
use safe lower level constructs.unsafe code
There is only one source file,
lib.rs
, so all uses of “unsafe” are found there.fn sun_path_offset()
: calculates the offset of a struct member using pointer arithmetic. Circumnavigates possible UB (cf. https://internals.rust-lang.org/t/9273/127).offsetof()
is still an unsolved problem in Rust.impl Drop for Inner
: obligatory dtor.fn Inner::new()
: wrapssocket(2)
; return value handled ok.fn Inner::new_pair()
: wrapssocketpair(2)
; return value handled ok; out parameter handled correctly.fn Inter::try_clone()
: wrapsdup(2)
; return value handled ok.fn Inner::shutdown()
: wrapsshutdown(2)
; return value handled ok; API safe by accepting an enum wrapper over theint how
argument.fn Inner::timeout()
: wrapsgetsockopt(2)
; return value handled ok; out parameter (struct timeval
) handled ok.fn Inner::set_timeout()
: wrapssetsockopt(2)
; return value handled ok; correct input validation, catching out of range values fortime_t
argument using saturating arithmetic.±
fn Inner::set_nonblocking()
: wrapsioctl(2)
withFIONBIO
; return value handled ok. Why not usefcntl(2)
withO_NONBLOCK
instead? Probably to save one syscall? This warrants an explantory comment.fn Inner::take_error()
: wrapsgetsockopt(2)
; return value handled ok; out parameter ok.unsafe fn sockaddr_un()
: initializes astruct sockaddr_un
; inputs validated appropriately; raw pointer access provably within bounds.fn SocketAddr::new()
: obtains a socket address; for use withgetpeername()
/getsockname()
etc.; return value handled ok (assuming the passed function returns the syscall return value).fn SocketAddr::address()
: casts memory of a wrapped type from[char]
to[u8]
; array bounds are preserved thus subsequent accesses return slices with valid bounds.fn UnixStream::connect()`: public API wrapping socket creation and
connect(2)``; arguments obtained from internal APIs deemed safe; return value handled ok.fn UnixStream::local_addr()`: wraps
getsockname(2); return value and arguments handled by
SocketAddr::new()``.fn UnixStream::peer_addr()`: wraps
getpeername(2); return value and arguments handled by
SocketAddr::new()``.impl Read for UnixStream, fn read()
: wrapsrecv(2)
; return value handled ok; argument values obtained from safe rust type.impl Write for UnixStream, fn write()
: wrapssend(2)
; return value handled ok; argument values obtained from safe rust type.impl FromRawFd for UnixStream, fn from_raw_fd()
: unsafe trait; constructs aUnixStream
without validating the arugment.±
fn UnixListener::bind()
: wraps socket creation,bind(2)
andlisten(2
; return values handled ok; backlog of socket queue hard-coded to the default Linux maximum of 128 which is reasonable but deserves mention in the docs.fn UnixListener::accept()
: wrapper foraccept(2)
; return value and argument checks deferred toSocketAddr::new()
.fn UnixListener::local_addr()
: wrapper forgetsockname(2)
; return value and arguments handled bySocketAddr::new()
.impl FromRawFd for UnixListener, fn from_raw_fd()
: unsafe trait, constructsUnixListener
without validating anything.fn UnixDatagram::bind()
: wrapsbind(2)
forSOCK_DGRAM
type sockets; return value handled ok; args obtained from safe wrappers.fn UnixDatagram::connect()
: wrapsconnect(2)
; return value handled ok; args obtained from safe wrappers.fn UnixDatagram::local_addr()`: wraps
getsockname(2); return value and arguments handled by
SocketAddr::new()``.fn UnixDatagram::peer_addr()`: wraps
getpeername(2); return value and arguments handled by
SocketAddr::new()``.fn UnixDatagram::recv_from()`: wraps
recvfrom(2)``; return value handled ok; args obtained from safe wrappers.fn UnixDatagram::recv()`: wraps
recv(2)``; args obtained from safe types; return value handled ok.fn UnixDatagram::send_to()`: wraps
send_to(2)``; args obtained from safe types; return value handled ok.fn UnixDatagram::send()`: wraps
send_to(2)``; args obtained from safe types; return value handled ok.impl FromRawFd for UnixDatagram, fn from_raw_fd()
: unsafe trait, constructsUnixDatagram
without validating anything.