Skip to content

fix: writing outside of sharedBuffer if size small enough - #313

Open
Dushistov wants to merge 1 commit into
couchbase:masterfrom
Dushistov:fix-shared-buffer-allocation
Open

Dushistov wants to merge 1 commit into
couchbase:masterfrom
Dushistov:fix-shared-buffer-allocation

Conversation

@Dushistov

Copy link
Copy Markdown
Contributor

valgrind catch in rather old couchbase-lite-core write of 8 bytes to place, for each was allocated only 4.

So that's why this fix.
Plus I suppose it is C++ standard violation to allocate less them sizeof(type):

The lifetime of an object of type T begins when:

    (2.1)
    storage with the proper alignment and size for type T is obtained

valgrind catch in rather old couchbase-lite-core write of 8 bytes
to place, for each was allocated only 4.

So that's why this fix.
Plus I suppose it is C++ standard violation to allocate less them sizeof(type):

```
The lifetime of an object of type T begins when:

    (2.1)
    storage with the proper alignment and size for type T is obtained
```
@snej

snej commented Oct 13, 2026

Copy link
Copy Markdown
Contributor

sharedBuffer::_buf is never actually used; it's just a placeholder that indicates where the variable-sized data starts. So there's no actual misbehavior at runtime if we allocate less space, although you're right that technically it's incorrect.

The best fix would be to change the size of _buf to 0. I am not sure if all our compilers will accept that, though.

@Dushistov

Copy link
Copy Markdown
Contributor Author

So there's no actual misbehavior at runtime

I suppose at least with modern gcc there is misbehaviour. valgrind catch write of 8 bytes by pointer that point to 4 bytes allocation.

The best fix would be to change the size of _buf to 0.

I suppose this is change nothing is modern allocators, where minimal bucket size is 8 bytes,
so even you allocate 4 bytes, really allocator give you at least 8 bytes, that's why I suppose this bug was not noticed before.

@snej

snej commented Oct 14, 2026 •

Copy link
Copy Markdown
Contributor

valgrind catch write of 8 bytes by pointer that point to 4 bytes allocation.

Are you sure? I think valgrind is complaining that sizeof(sharedBuffer) is 8, but only 4 bytes are allocated. While this is invalid by the spec, it's not a problem at runtime because only 4 bytes (the refcount) are ever written to the heap block.

even you allocate 4 bytes, really allocator give you at least 8 bytes

The allocator will let you get away with this, but AddressSanitizer will catch writing even 1 byte past the size passed to malloc. That's why I don't think there is any problem at runtime.

@Dushistov

Dushistov commented Oct 15, 2026 •

Copy link
Copy Markdown
Contributor Author

Are you sure?

Yes, here is relevant part of valgrind's log:

Error Invalid write of size 8
 Info main stack trace (user code at the bottom)
      at FLSliceResult_New (FLSlice.cc:154)
      at alloc_slice (slice.hh:800)
      at litecore::blip::MessageIn::receivedFrame(litecore::blip::Codec&, fleece::slice, litecore::blip::FrameFlags) (Message.cc:200)
 Info Address 0x8484060 is 0 bytes inside a block of size 4 alloc'd
      at malloc (vg_replace_malloc.c:446)
      at operator new (FLSlice.cc:120)
      at FLSliceResult_New (FLSlice.cc:154)

Error Invalid write of size 8
 Info main stack trace (user code at the bottom)
      at FLSliceResult_New (FLSlice.cc:154)
      at alloc_slice (slice.hh:800)
      at fleece::Writer::copyOutput() const (Writer.cc:231)
      at fleece::Writer::finish() (Writer.cc:242)
 Info Address 0x84840b0 is 0 bytes inside a block of size 4 alloc'd
      at malloc (vg_replace_malloc.c:446)
      at operator new (FLSlice.cc:120)
      at FLSliceResult_New (FLSlice.cc:154)

The allocator will let you get away with this, but AddressSanitizer will catch writing even 1 byte past the size passed to malloc

Yeah, but I catched this bug only on network exchange, so real users, that build code obviously without AddressSanitizer, can not see this bug, even if it exists, because of allocator gives them 8, 16 or more bytes.

But the main point, that uint8_t _buf[0]; and malloc(4) is optimizations that gives nothing, because of the real allocators still gives you 8 or more bytes, so why have UB in code that gives nothing?

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants