Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fixed a potential bug that could cause a segment fault when insert a large tuple. #716

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

CodeZHXS
Copy link

Statement

In the file src/storage/page/table_page.cpp, Here is a TablePage::GetNextTupleOffset function.

  // Warning: slot_end_offset may be smaller than tuple.GetLength()
  auto tuple_offset = slot_end_offset - tuple.GetLength();

  auto offset_size = TABLE_PAGE_HEADER_SIZE + TUPLE_INFO_SIZE * (num_tuples_ + 1);
  if (tuple_offset < offset_size) {
    return std::nullopt;
  }

Since tuple_offset is unsigned long, tuple_offset will be greater than the value of offset_size after underflow occurs. Therefore, the condition of if clause is not satisfied, we will return an error tuple_offset(the value of tuple_offset exceeds the page size). When we insert a tuple at this position, a segment fault occurs.

How to hack it

I reproduced this fault using BusTub Web Shell(Fall 2023).

First, create a table with larger schema.

CREATE TABLE my_table (
    myname VARCHAR(2500),
);

Then, insert some tuples into it. In this example, I inserted $3$ tuples of $2500$ bytes long.

INSERT INTO my_table VALUES ('chenyujie|chenyujie|chenyujie|chenyujie|chenyujie|chenyujie|chenyujie|chenyujie|chenyujie|chenyujie|chenyujie|chenyujie|chenyujie|chenyujie|chenyujie|chenyujie|chenyujie|chenyujie|chenyujie|chenyujie|chenyujie|chenyujie|chenyujie|chenyujie|chenyujie|chenyujie|chenyujie|chenyujie|chenyujie|chenyujie|chenyujie|chenyujie|chenyujie|chenyujie|chenyujie|chenyujie|chenyujie|chenyujie|chenyujie|chenyujie|chenyujie|chenyujie|chenyujie|chenyujie|chenyujie|chenyujie|chenyujie|chenyujie|chenyujie|chenyujie|chenyujie|chenyujie|chenyujie|chenyujie|chenyujie|chenyujie|chenyujie|chenyujie|chenyujie|chenyujie|chenyujie|chenyujie|chenyujie|chenyujie|chenyujie|chenyujie|chenyujie|chenyujie|chenyujie|chenyujie|chenyujie|chenyujie|chenyujie|chenyujie|chenyujie|chenyujie|chenyujie|chenyujie|chenyujie|chenyujie|chenyujie|chenyujie|chenyujie|chenyujie|chenyujie|chenyujie|chenyujie|chenyujie|chenyujie|chenyujie|chenyujie|chenyujie|chenyujie|chenyujie|chenyujie|chenyujie|chenyujie|chenyujie|chenyujie|chenyujie|chenyujie|chenyujie|chenyujie|chenyujie|chenyujie|chenyujie|chenyujie|chenyujie|chenyujie|chenyujie|chenyujie|chenyujie|chenyujie|chenyujie|chenyujie|chenyujie|chenyujie|chenyujie|chenyujie|chenyujie|chenyujie|chenyujie|chenyujie|chenyujie|chenyujie|chenyujie|chenyujie|chenyujie|chenyujie|chenyujie|chenyujie|chenyujie|chenyujie|chenyujie|chenyujie|chenyujie|chenyujie|chenyujie|chenyujie|chenyujie|chenyujie|chenyujie|chenyujie|chenyujie|chenyujie|chenyujie|chenyujie|chenyujie|chenyujie|chenyujie|chenyujie|chenyujie|chenyujie|chenyujie|chenyujie|chenyujie|chenyujie|chenyujie|chenyujie|chenyujie|chenyujie|chenyujie|chenyujie|chenyujie|chenyujie|chenyujie|chenyujie|chenyujie|chenyujie|chenyujie|chenyujie|chenyujie|chenyujie|chenyujie|chenyujie|chenyujie|chenyujie|chenyujie|chenyujie|chenyujie|chenyujie|chenyujie|chenyujie|chenyujie|chenyujie|chenyujie|chenyujie|chenyujie|chenyujie|chenyujie|chenyujie|chenyujie|chenyujie|chenyujie|chenyujie|chenyujie|chenyujie|chenyujie|chenyujie|chenyujie|chenyujie|chenyujie|chenyujie|chenyujie|chenyujie|chenyujie|chenyujie|chenyujie|chenyujie|chenyujie|chenyujie|chenyujie|chenyujie|chenyujie|chenyujie|chenyujie|chenyujie|chenyujie|chenyujie|chenyujie|chenyujie|chenyujie|chenyujie|chenyujie|chenyujie|chenyujie|chenyujie|chenyujie|chenyujie|chenyujie|chenyujie|chenyujie|chenyujie|chenyujie|chenyujie|chenyujie|chenyujie|chenyujie|chenyujie|chenyujie|chenyujie|chenyujie|chenyujie|chenyujie|chenyujie|chenyujie|chenyujie|chenyujie|chenyujie|chenyujie|');

INSERT INTO my_table VALUES ('chenyujie|chenyujie|chenyujie|chenyujie|chenyujie|chenyujie|chenyujie|chenyujie|chenyujie|chenyujie|chenyujie|chenyujie|chenyujie|chenyujie|chenyujie|chenyujie|chenyujie|chenyujie|chenyujie|chenyujie|chenyujie|chenyujie|chenyujie|chenyujie|chenyujie|chenyujie|chenyujie|chenyujie|chenyujie|chenyujie|chenyujie|chenyujie|chenyujie|chenyujie|chenyujie|chenyujie|chenyujie|chenyujie|chenyujie|chenyujie|chenyujie|chenyujie|chenyujie|chenyujie|chenyujie|chenyujie|chenyujie|chenyujie|chenyujie|chenyujie|chenyujie|chenyujie|chenyujie|chenyujie|chenyujie|chenyujie|chenyujie|chenyujie|chenyujie|chenyujie|chenyujie|chenyujie|chenyujie|chenyujie|chenyujie|chenyujie|chenyujie|chenyujie|chenyujie|chenyujie|chenyujie|chenyujie|chenyujie|chenyujie|chenyujie|chenyujie|chenyujie|chenyujie|chenyujie|chenyujie|chenyujie|chenyujie|chenyujie|chenyujie|chenyujie|chenyujie|chenyujie|chenyujie|chenyujie|chenyujie|chenyujie|chenyujie|chenyujie|chenyujie|chenyujie|chenyujie|chenyujie|chenyujie|chenyujie|chenyujie|chenyujie|chenyujie|chenyujie|chenyujie|chenyujie|chenyujie|chenyujie|chenyujie|chenyujie|chenyujie|chenyujie|chenyujie|chenyujie|chenyujie|chenyujie|chenyujie|chenyujie|chenyujie|chenyujie|chenyujie|chenyujie|chenyujie|chenyujie|chenyujie|chenyujie|chenyujie|chenyujie|chenyujie|chenyujie|chenyujie|chenyujie|chenyujie|chenyujie|chenyujie|chenyujie|chenyujie|chenyujie|chenyujie|chenyujie|chenyujie|chenyujie|chenyujie|chenyujie|chenyujie|chenyujie|chenyujie|chenyujie|chenyujie|chenyujie|chenyujie|chenyujie|chenyujie|chenyujie|chenyujie|chenyujie|chenyujie|chenyujie|chenyujie|chenyujie|chenyujie|chenyujie|chenyujie|chenyujie|chenyujie|chenyujie|chenyujie|chenyujie|chenyujie|chenyujie|chenyujie|chenyujie|chenyujie|chenyujie|chenyujie|chenyujie|chenyujie|chenyujie|chenyujie|chenyujie|chenyujie|chenyujie|chenyujie|chenyujie|chenyujie|chenyujie|chenyujie|chenyujie|chenyujie|chenyujie|chenyujie|chenyujie|chenyujie|chenyujie|chenyujie|chenyujie|chenyujie|chenyujie|chenyujie|chenyujie|chenyujie|chenyujie|chenyujie|chenyujie|chenyujie|chenyujie|chenyujie|chenyujie|chenyujie|chenyujie|chenyujie|chenyujie|chenyujie|chenyujie|chenyujie|chenyujie|chenyujie|chenyujie|chenyujie|chenyujie|chenyujie|chenyujie|chenyujie|chenyujie|chenyujie|chenyujie|chenyujie|chenyujie|chenyujie|chenyujie|chenyujie|chenyujie|chenyujie|chenyujie|chenyujie|chenyujie|chenyujie|chenyujie|chenyujie|chenyujie|chenyujie|chenyujie|chenyujie|chenyujie|chenyujie|chenyujie|chenyujie|chenyujie|chenyujie|chenyujie|chenyujie|');

INSERT INTO my_table VALUES ('chenyujie|chenyujie|chenyujie|chenyujie|chenyujie|chenyujie|chenyujie|chenyujie|chenyujie|chenyujie|chenyujie|chenyujie|chenyujie|chenyujie|chenyujie|chenyujie|chenyujie|chenyujie|chenyujie|chenyujie|chenyujie|chenyujie|chenyujie|chenyujie|chenyujie|chenyujie|chenyujie|chenyujie|chenyujie|chenyujie|chenyujie|chenyujie|chenyujie|chenyujie|chenyujie|chenyujie|chenyujie|chenyujie|chenyujie|chenyujie|chenyujie|chenyujie|chenyujie|chenyujie|chenyujie|chenyujie|chenyujie|chenyujie|chenyujie|chenyujie|chenyujie|chenyujie|chenyujie|chenyujie|chenyujie|chenyujie|chenyujie|chenyujie|chenyujie|chenyujie|chenyujie|chenyujie|chenyujie|chenyujie|chenyujie|chenyujie|chenyujie|chenyujie|chenyujie|chenyujie|chenyujie|chenyujie|chenyujie|chenyujie|chenyujie|chenyujie|chenyujie|chenyujie|chenyujie|chenyujie|chenyujie|chenyujie|chenyujie|chenyujie|chenyujie|chenyujie|chenyujie|chenyujie|chenyujie|chenyujie|chenyujie|chenyujie|chenyujie|chenyujie|chenyujie|chenyujie|chenyujie|chenyujie|chenyujie|chenyujie|chenyujie|chenyujie|chenyujie|chenyujie|chenyujie|chenyujie|chenyujie|chenyujie|chenyujie|chenyujie|chenyujie|chenyujie|chenyujie|chenyujie|chenyujie|chenyujie|chenyujie|chenyujie|chenyujie|chenyujie|chenyujie|chenyujie|chenyujie|chenyujie|chenyujie|chenyujie|chenyujie|chenyujie|chenyujie|chenyujie|chenyujie|chenyujie|chenyujie|chenyujie|chenyujie|chenyujie|chenyujie|chenyujie|chenyujie|chenyujie|chenyujie|chenyujie|chenyujie|chenyujie|chenyujie|chenyujie|chenyujie|chenyujie|chenyujie|chenyujie|chenyujie|chenyujie|chenyujie|chenyujie|chenyujie|chenyujie|chenyujie|chenyujie|chenyujie|chenyujie|chenyujie|chenyujie|chenyujie|chenyujie|chenyujie|chenyujie|chenyujie|chenyujie|chenyujie|chenyujie|chenyujie|chenyujie|chenyujie|chenyujie|chenyujie|chenyujie|chenyujie|chenyujie|chenyujie|chenyujie|chenyujie|chenyujie|chenyujie|chenyujie|chenyujie|chenyujie|chenyujie|chenyujie|chenyujie|chenyujie|chenyujie|chenyujie|chenyujie|chenyujie|chenyujie|chenyujie|chenyujie|chenyujie|chenyujie|chenyujie|chenyujie|chenyujie|chenyujie|chenyujie|chenyujie|chenyujie|chenyujie|chenyujie|chenyujie|chenyujie|chenyujie|chenyujie|chenyujie|chenyujie|chenyujie|chenyujie|chenyujie|chenyujie|chenyujie|chenyujie|chenyujie|chenyujie|chenyujie|chenyujie|chenyujie|chenyujie|chenyujie|chenyujie|chenyujie|chenyujie|chenyujie|chenyujie|chenyujie|chenyujie|chenyujie|chenyujie|chenyujie|chenyujie|chenyujie|chenyujie|chenyujie|chenyujie|chenyujie|chenyujie|chenyujie|chenyujie|chenyujie|chenyujie|chenyujie|chenyujie|');

Then, You should be able to observe that the program has failed.
pic

@skyzh
Copy link
Member

skyzh commented Jun 25, 2024

I think this would cause a dead loop given the outer function would continuously create new pages?

@CodeZHXS
Copy link
Author

I think this would cause a dead loop given the outer function would continuously create new pages?

The outer function is TableHeap::InsertTuple, which is in src/storage/table/table_heap.cpp. This function will refuse to insert a tuple larger than page size instead of creating pages in dead loop.

In my cases, the length of a tuple is $2500$ bytes. This should be legal, because I can store a tuple in a page. But due to the problem I mentioned above, the program has an error. You can run my test sample in the web shell or in your locally compiled shell.

So we have to add a check for tuple_offset (As I commit).

@UnpureRationalist
Copy link
Contributor

I also came into this bug in my experiments. The reason is the evil unsigned number arithmetic: when the length of inserted tuple(tuple.GetLength()) is larger than slot_end_offset, then auto tuple_offset = slot_end_offset - tuple.GetLength(); will be a extremely large postive number, then memory error is caused. The PR LGTM and can avoid this bug.

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.

3 participants