Skip to content

tuple: fix crash on hashing tuple with double fields#10098

Merged
locker merged 1 commit intotarantool:masterfrom
locker:tuple-hash-double-fix
Jun 7, 2024
Merged

tuple: fix crash on hashing tuple with double fields#10098
locker merged 1 commit intotarantool:masterfrom
locker:tuple-hash-double-fix

Conversation

@locker
Copy link
Member

@locker locker commented Jun 6, 2024

tuple_hash_field() doesn't advance the MsgPack cursor after hashing a tuple field with the type double, which can result in crashes both in memtx (while inserting a tuple into a hash index) and in vinyl (while writing a bloom filter on dump or compaction).

The bug was introduced by commit 51af059.

Closes #10090

@locker locker requested a review from a team as a code owner June 6, 2024 08:04
`tuple_hash_field()` doesn't advance the MsgPack cursor after hashing
a tuple field with the type `double`, which can result in crashes both
in memtx (while inserting a tuple into a hash index) and in vinyl
(while writing a bloom filter on dump or compaction).

The bug was introduced by commit 51af059 ("box: compare and hash
msgpack value of double key field as double").

Closes tarantool#10090

NO_DOC=bug fix
@locker locker force-pushed the tuple-hash-double-fix branch from b902622 to 58dd9e8 Compare June 6, 2024 08:16
@coveralls
Copy link

Coverage Status

coverage: 87.087% (-0.02%) from 87.107%
when pulling 58dd9e8 on locker:tuple-hash-double-fix
into 97a801e
on tarantool:master
.

@locker locker requested a review from mkostoevr June 6, 2024 08:36
Copy link
Contributor

@mkostoevr mkostoevr left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Very good catch, thanks. LGTM.

BTW I like how it introduces 70 times more tests than changed code.

@locker locker assigned locker and unassigned mkostoevr Jun 6, 2024
@locker locker added the full-ci Enables all tests for a pull request label Jun 7, 2024
@locker locker merged commit bc0daf9 into tarantool:master Jun 7, 2024
@locker locker deleted the tuple-hash-double-fix branch June 7, 2024 09:02
@locker
Copy link
Member Author

locker commented Jun 7, 2024

Cherry-picked to 2.11 and 3.1.

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

Labels

full-ci Enables all tests for a pull request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

src/box/tuple_hash.cc:317: uint32_t tuple_hash_field(uint32_t*, uint32_t*, const char**, field_type, coll*): Assertion `0' failed.

4 participants