-
Notifications
You must be signed in to change notification settings - Fork 1.1k
Fix potential malformed headers in parquet delta decoder #22275
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
Changes from all commits
6a32d88
23aade2
1a52a6d
66df3a3
95f3d8a
c839f1b
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -366,9 +366,11 @@ CUDF_KERNEL void __launch_bounds__(decode_delta_binary_block_size) | |
| block.sync(); | ||
|
|
||
| auto const batch_size = db->values_per_mb; | ||
| if (batch_size > max_delta_mini_block_size) { | ||
| set_error(static_cast<kernel_error::value_type>(decode_error::DELTA_PARAMS_UNSUPPORTED), | ||
| error_code); | ||
| if (db->error or batch_size > max_delta_mini_block_size) { | ||
|
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Check for |
||
| if (block.thread_rank() == 0) { | ||
| set_error(static_cast<kernel_error::value_type>(decode_error::DELTA_PARAMS_UNSUPPORTED), | ||
| error_code); | ||
| } | ||
| return; | ||
| } | ||
|
|
||
|
|
@@ -546,6 +548,15 @@ CUDF_KERNEL void __launch_bounds__(decode_block_size) | |
| } | ||
| block.sync(); | ||
|
|
||
| // Propagate malformed-header errors from either underlying DELTA_BINARY_PACKED decoder. | ||
| if (prefix_db->error or suffix_db->error) { | ||
|
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Same everywhere |
||
| if (block.thread_rank() == 0) { | ||
| set_error(static_cast<kernel_error::value_type>(decode_error::DELTA_PARAMS_UNSUPPORTED), | ||
| error_code); | ||
| } | ||
| return; | ||
| } | ||
|
|
||
| // assert that prefix and suffix have same mini-block size | ||
| if (prefix_db->values_per_mb != suffix_db->values_per_mb or | ||
| prefix_db->block_size != suffix_db->block_size or | ||
|
|
@@ -562,8 +573,10 @@ CUDF_KERNEL void __launch_bounds__(decode_block_size) | |
| // sanity check to make sure we can process this page | ||
| auto const batch_size = prefix_db->values_per_mb; | ||
| if (batch_size > max_delta_mini_block_size) { | ||
| set_error(static_cast<kernel_error::value_type>(decode_error::DELTA_PARAMS_UNSUPPORTED), | ||
| error_code); | ||
| if (block.thread_rank() == 0) { | ||
|
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Only one thread needs to set the error |
||
| set_error(static_cast<kernel_error::value_type>(decode_error::DELTA_PARAMS_UNSUPPORTED), | ||
| error_code); | ||
| } | ||
| return; | ||
| } | ||
|
|
||
|
|
@@ -759,14 +772,18 @@ CUDF_KERNEL void __launch_bounds__(decode_block_size) | |
| } | ||
| block.sync(); | ||
|
|
||
| int const leaf_level_index = s->col.max_nesting_depth - 1; | ||
|
|
||
| // sanity check to make sure we can process this page | ||
| auto const batch_size = db->values_per_mb; | ||
| if (batch_size > max_delta_mini_block_size) { | ||
| set_error(static_cast<int32_t>(decode_error::DELTA_PARAMS_UNSUPPORTED), error_code); | ||
| if (db->error or batch_size > max_delta_mini_block_size) { | ||
| if (block.thread_rank() == 0) { | ||
| set_error(static_cast<kernel_error::value_type>(decode_error::DELTA_PARAMS_UNSUPPORTED), | ||
| error_code); | ||
| } | ||
| return; | ||
| } | ||
|
|
||
| int const leaf_level_index = s->col.max_nesting_depth - 1; | ||
|
|
||
| // db->init_binary_block below resets db->values_per_mb | ||
| block.sync(); | ||
| // if this is a bounds page, then we need to decode up to the first mini-block | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Validate header here and set
errorfield.