Skip to content

[operators] Guard max-pooling-nhwc padding arithmetic overflow - #11409

Open
anagnorisis2peripeteia wants to merge 1 commit into
google:masterfrom
anagnorisis2peripeteia:maxpool-dimension-followup
Open

anagnorisis2peripeteia wants to merge 1 commit into
google:masterfrom
anagnorisis2peripeteia:maxpool-dimension-followup

Conversation

@anagnorisis2peripeteia

Copy link
Copy Markdown
Contributor

Summary

reshape_max_pooling2d_nhwc computed the effective kernel size, the TF-SAME
padding, and the padded input size with raw size_t arithmetic.

  • A large pooling_window * dilation product wraps the effective kernel to a
    small value, so the output dimension is computed from a wrapped padded input
    size.
  • The indirection buffer then addresses input pixels outside the input tensor,
    which is an out-of-bounds read at run time.
  • TF-SAME padding was narrowed to uint32_t with no range check.

What changed

  • Compute the effective kernel height and width with checked multiply and add.
  • Compute the TF-SAME scaled output size and total padding with checked
    arithmetic, and reject padding above UINT32_MAX.
  • Compute the padded input size with checked addition before the output
    dimension calculation.
  • Return xnn_status_out_of_memory and log the failing term on each overflow.

Testing

  • max-pooling-nhwc-test passes, including the new
    padded_input_height_overflow case.

Notes

This is independent of #11270, which covers the max-pooling stride arithmetic
in the unpooling path. This PR covers the padding path only.

Reshape computed the effective kernel size, TF-SAME padding, and the
padded input size with raw size_t arithmetic. A large pooling window
times dilation wraps the effective kernel to a small value, so the
output dimension is computed from a wrapped padded input size and the
indirection buffer addresses input pixels outside the input tensor.

Compute each term with checked helpers, and reject padding that does not
fit the uint32_t operator fields.
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.

1 participant