Skip to content

Only grow wasm memory with write ops#3315

Merged
brentstone merged 3 commits intomainfrom
tiago/wasmer-read-only-mem
Jun 6, 2024
Merged

Only grow wasm memory with write ops#3315
brentstone merged 3 commits intomainfrom
tiago/wasmer-read-only-mem

Conversation

@sug0
Copy link
Copy Markdown
Collaborator

@sug0 sug0 commented May 27, 2024 •
edited
Loading

Describe your changes

Closes #3314

Indicate on which release or other PRs this topic is based on

Based on v0.38.1

Checklist before merging to draft

  • I have added a changelog
  • Git history is in acceptable state

@sug0 sug0 added ledger wasm breaking:state State breaking change that is not backwards compatible with a state recorded by current version labels May 27, 2024
@sug0 sug0 changed the title Tiago/wasmer read only mem Only grow wasm memory with write ops May 27, 2024
@codecov
Copy link
Copy Markdown

codecov bot commented May 27, 2024 •
edited
Loading

Codecov Report

Attention: Patch coverage is 95.83333% with 4 lines in your changes missing coverage. Please review.

Project coverage is 54.07%. Comparing base (883bd0f) to head (e265807).

Files Patch % Lines
crates/namada/src/vm/wasm/memory.rs 95.00% 2 Missing ⚠️
crates/namada/src/vm/wasm/run.rs 96.42% 2 Missing ⚠️
Additional details and impacted files 
@@            Coverage Diff             @@
##             main    #3315      +/-   ##
==========================================
+ Coverage   54.05%   54.07%   +0.01%     
==========================================
  Files         315      315              
  Lines      106296   106344      +48     
==========================================
+ Hits        57461    57505      +44     
- Misses      48835    48839       +4

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@sug0
Copy link
Copy Markdown
Collaborator Author

sug0 commented May 28, 2024 •
edited
Loading

#3308 (comment)
scrapped, too complex to write without using refcells or smth. might as well keep passing &mut refs to a store, which is never accessed mutably anyway

This was referenced May 28, 2024
Merged
Closed
@brentstone brentstone mentioned this pull request May 31, 2024
Closed
@sug0 sug0 force-pushed the tiago/wasmer-read-only-mem branch from 44cb3d0 to 76628c5 Compare June 2, 2024 09:28
@sug0 sug0 marked this pull request as ready for review June 2, 2024 09:30
@sug0 sug0 requested review from batconjurer, grarco and tzemanovic June 2, 2024 09:31
Fraccaman added a commit that referenced this pull request Jun 3, 2024
@Fraccaman
Merge branch 'tiago/wasmer-read-only-mem' (#3315)
75e0fe4 
* up/tiago/wasmer-read-only-mem:
  Changelog for #3315
  Test that RO wasm memory cannot grow
  Avoid growing wasm memory when performing reads
brentstone added a commit that referenced this pull request Jun 5, 2024
@brentstone
Merge branch 'tiago/wasmer-read-only-mem' (#3315)
6fb4f0f 
* origin/tiago/wasmer-read-only-mem:
  Changelog for #3315
  Test that RO wasm memory cannot grow
  Avoid growing wasm memory when performing reads
@brentstone brentstone merged commit 544319b into main Jun 6, 2024
@brentstone brentstone deleted the tiago/wasmer-read-only-mem branch June 6, 2024 07:53
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@tzemanovic tzemanovic tzemanovic approved these changes

@grarco grarco grarco approved these changes

@batconjurer batconjurer Awaiting requested review from batconjurer

Assignees

No one assigned

Labels

breaking:state State breaking change that is not backwards compatible with a state recorded by current version ledger wasm

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Avoid growing wasm memory when performing out-of-bounds reads

5 participants

@sug0 @tzemanovic @grarco @Fraccaman @brentstone