sn/storage: Fix payload overflow in combined FSTree.Get()#3801
sn/storage: Fix payload overflow in combined FSTree.Get()#3801roman-khimov merged 1 commit intomasterfrom
FSTree.Get()#3801Conversation
5e1f2c8 to
5908cc2
Compare
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #3801 +/- ##
==========================================
+ Coverage 25.52% 25.55% +0.02%
==========================================
Files 661 661
Lines 42641 42647 +6
==========================================
+ Hits 10884 10897 +13
+ Misses 30753 30748 -5
+ Partials 1004 1002 -2 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
roman-khimov
left a comment
There was a problem hiding this comment.
- Not sure why this wasn't noticed previously. We have a lot of tests and production networks.
- IIUC we can have excessive data written into combined file already from write cache (not via regular writes). Which changes the game somewhat, technically the data is still here and your fix as it is now would work fine.
- Maybe payload-sized LimitReader can be applied layer above.
- Not sure if we need a tool to fix it for existing fstrees.
noticed by chance while reading this part. Problem was instantly reproduced in unit test, but I didn't try it in devenv. I also didn't see any way the service could mitigate this issue, such as by limiting reading |
5908cc2 to
3d4257a
Compare
Not true, WC doesn't have combined files, so it produces clean buffers that can be written correctly. This means we don't have broken data on disk, which is good. |
|
reproduced in devenv |
3d4257a to
ae546bc
Compare
To avoid exceeding the payload's boundaries, all callers would have to perform checks on their side. Although it's more natural to just read until EOF, and this is what's happening now. Signed-off-by: Leonard Lyubich <leonard@morphbits.io>
ae546bc to
0d1c686
Compare
i did not try to reproduce this on a cluster. For example, it might show up with
neofs-node/pkg/local_object_storage/writecache/flush.go
Line 224 in 58f146c