Skip to content

GitLab

  • Projects
  • Groups
  • Snippets
  • Help
    • Loading...
  • Help
    • Help
    • Support
    • Community forum
    • Submit feedback
    • Contribute to GitLab
  • Sign in
S
syscall
  • Project overview
    • Project overview
    • Details
    • Activity
    • Releases
  • Repository
    • Repository
    • Files
    • Commits
    • Branches
    • Tags
    • Contributors
    • Graph
    • Compare
  • Issues 8
    • Issues 8
    • List
    • Boards
    • Labels
    • Service Desk
    • Milestones
  • Merge Requests 9
    • Merge Requests 9
  • CI / CD
    • CI / CD
    • Pipelines
    • Jobs
    • Schedules
  • Operations
    • Operations
    • Incidents
    • Environments
  • Packages & Registries
    • Packages & Registries
    • Container Registry
  • Analytics
    • Analytics
    • CI / CD
    • Repository
    • Value Stream
  • Wiki
    • Wiki
  • Snippets
    • Snippets
  • Members
    • Members
  • Collapse sidebar
  • Activity
  • Graph
  • Create a new issue
  • Jobs
  • Commits
  • Issue Boards
  • redox-os
  • syscall
  • Issues
  • #29

Closed
Open
Opened Oct 13, 2019 by Dawid Ciężarkiewicz@dpc

Unsafe APIs

Hi,

I'm looking at the crev review of redox_syscall: https://github.com/MaulingMonkey/crev-proofs/commit/0e29470384492587074e19a2a1ff7adc341bea25 , pasted inline:

version: -1
date: "2019-07-24T14:53:11.836480900-07:00"
from:
  id-type: crev
  id: 6OZqHXqyUAF57grEY7IVMjRljdd9dgDxiNtr1NF1BdY
  url: "https://github.com/MaulingMonkey/crev-proofs"
package:
  source: "https://crates.io"
  name: redox_syscall
  version: 0.1.56
  revision: e3fd644ba9830d104c309f77c36dc6b94f92f2b1
  digest: FTw1q2J_JAvJHFyP4Xn0URVlkBDh1xI3mxs5sFn_A-U
review:
  thoroughness: low
  understanding: low
  rating: negative
comment: |
  Exposes unsound APIs, lots of unverified syscalls.
  
  Reviewed:
  
    src\arch\*.rs:  Skimmed... looks reasonable, but didn't verify correct instructions / register invalidation.
  
    src\io\dma.rs:  Some unsafe... looks correct, but not thoroughly tested.
    src\io\io.rs:   +1
    src\io\mmio.rs: UNSOUND (can construct uninitialized() T via "safe" `Mmio::new()`!)
    src\io\mod.rs:  +1
    src\io\pio.rs:  Some unsafe... looks reasonable, but didn't verify correct instructions.
  
    src\scheme\generate.sh: +1
    src\scheme\mod.rs:      +1
    src\scheme\scheme*.rs:  UNSOUND (can construct arbitrary slices from arbitrary Packet s via `Scheme*::handle`)
  
    .cargo_vcs_info.json: +1
    .cargo-ok:            +1
    .gitlab-ci.yml:       +1
    Cargo.toml:           +1
    Cargo.toml.orig:      +1
    LICENSE:              +1
    README.md:            +1
  
  Skimmed:
  
    src\call.rs:    Lots of unsafe syscalls... unverified.
    src\data.rs:    UNSOUND (Map deref etc.)
    src\error.rs:   No tests for STR_ERROR, but at least it's sound.
    src\flag.rs:    Sound, magic constant city, meh.
    src\lib.rs:     LEAKS UNSOUND TRAITS into public interface!
    src\number.rs:  Safe, magic constant city.
    src\tests.rs:   Unsafe, but only #[test]s

I just wanted to bring it to your attention. I guess in a low-level crate like this it might be more complicated to judge the unsafe issues. Please let me know what you think. Are some of the unsafety issues real? Is there anything that can be improved about it?

Assignee
Assign to
None
Milestone
None
Assign milestone
Time tracking
None
Due date
None
Reference: redox-os/syscall#29