Skip to content

Commit

Permalink
Issue percona#15: SIGSEGV from reading in blob data
Browse files Browse the repository at this point in the history
- convert_record_from_storage_format(buf) worked incorrectly
  when buf != table->record[0]. Length bytes were written
  to table->record[0] still.  Moved field->move_field_offset()
  call earlier to prevent that.
- Also, added comments and free() calls to free the retrieved
  blobs (the API is undocumented so we copy what InnoDB does).
  • Loading branch information
spetrunia authored and jtolmer committed Jan 5, 2016
1 parent ec0a85c commit 8d940b5
Show file tree
Hide file tree
Showing 2 changed files with 27 additions and 3 deletions.
15 changes: 13 additions & 2 deletions storage/rocksdb/ha_rocksdb.cc
Original file line number Diff line number Diff line change
Expand Up @@ -1073,16 +1073,20 @@ int ha_rocksdb::convert_record_from_storage_format(uchar * buf)
const char *data_len_str;
if (!(data_len_str= reader.read(length_bytes)))
return HA_ERR_INTERNAL_ERROR;

blob->move_field_offset(ptr_diff);
memcpy(blob->ptr, data_len_str, length_bytes);

uint32 data_len= blob->get_length((uchar*)data_len_str, length_bytes,
table->s->db_low_byte_first);
const char *blob_ptr;
if (!(blob_ptr= reader.read(data_len)))
{
blob->move_field_offset(-ptr_diff);
return HA_ERR_INTERNAL_ERROR;
}

blob->move_field_offset(ptr_diff);
// set 8-byte pointer to 0, like innodb does.
// set 8-byte pointer to 0, like innodb does (relevant for 32-bit platforms)
memset(blob->ptr + length_bytes, 0, 8);
memcpy(blob->ptr + length_bytes, &blob_ptr, sizeof(uchar**));
blob->move_field_offset(-ptr_diff);
Expand Down Expand Up @@ -2836,6 +2840,13 @@ int ha_rocksdb::extra(enum ha_extra_function operation)
case HA_EXTRA_NO_KEYREAD:
keyread_only= FALSE;
break;
case HA_EXTRA_FLUSH:
/*
If the table has blobs, then they are part of retrieved_record. This call
invalidates them.
*/
retrieved_record.clear();
break;
default:
break;
}
Expand Down
15 changes: 14 additions & 1 deletion storage/rocksdb/ha_rocksdb.h
Original file line number Diff line number Diff line change
Expand Up @@ -122,7 +122,13 @@ class ha_rocksdb: public handler
/* Buffer used by convert_record_to_storage_format() */
String storage_record;

/* Last retrieved record, in table->record[0] data format */
/*
Last retrieved record, in table->record[0] data format. It also serves as
as storage for blob data (Field_blob object have pointers to here)
TODO: Dont we lose one malloc() per record read by having it as std::string
instead of rocksdb::Slice?
*/
std::string retrieved_record;

/* If TRUE, reads should place locks on rows */
Expand Down Expand Up @@ -312,6 +318,13 @@ class ha_rocksdb: public handler
int delete_all_rows(void);
int truncate();

int reset()
{
/* Free blob data */
retrieved_record.clear();
return 0;
}

void remove_rows(RDBSE_TABLE_DEF *tbl);
ha_rows records_in_range(uint inx, key_range *min_key,
key_range *max_key);
Expand Down

0 comments on commit 8d940b5

Please sign in to comment.