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

BUG: interpolate/ndbspline: fix OOB access for len(tx) != len(ty) in tensor product splines #19581

Merged
merged 1 commit into from Nov 26, 2023

Conversation

ev-br
Copy link
Member

@ev-br ev-br commented Nov 23, 2023

To be able to release the GIL, we pack the tuple of 1D knot arrays into a 2D array. Since the lengths of knot arrays for different dimensions may differ, the 2D array has dimensions (ndim, max_length), with extra elements filled with NaNs.

The actual lengths of knot arrays need to be known to the interval search routine, which finds for a given x, the index i such that t[i] <= x < x[i+1]. Otherwise these extra nans confuse the interval search routine, leading to OOB access down the line.

…product splines

To be able to release the GIL, we pack the tuple of 1D knot arrays
into a 2D array. Since the lengths of knot arrays for different
dimensions may differ, the 2D array has dimensions
(ndim, max_length), with extra elements filled with NaNs.

The actual lengths of knot arrays need to be known to the interval
search routine, which finds for a given `x`, the index `i` such that
`t[i] <= x < x[i+1]`.
Otherwise these extra nans confuse the interval search routine, leading
to OOB access down the line.
@ev-br ev-br added defect A clear bug or issue that prevents SciPy from being installed or used as expected scipy.interpolate labels Nov 23, 2023
@ev-br ev-br added this to the 1.12.0 milestone Nov 23, 2023
@ev-br
Copy link
Member Author

ev-br commented Nov 23, 2023

Have tentatively added the 1.12 milestone, on the grounds of fixing a bug in a routine which itself is new in 1.12. OTOH not a big deal if it does not make it to 1.12.0.

Copy link
Contributor

@tylerjereddy tylerjereddy left a comment

Choose a reason for hiding this comment

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

I did a quick check that the regression test fails for me on latest main, and beyond that I'm happy to merge given that:

  • no old tests were changed, just a new regression test added
  • CI is passing, the 1 failure is unrelated
  • if this does cause issues, I'm sure Evgeni will help me patch it for a subsequent release
  • we're aiming to branch soon-ish

@tylerjereddy tylerjereddy merged commit f016aa9 into scipy:main Nov 26, 2023
22 of 23 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
defect A clear bug or issue that prevents SciPy from being installed or used as expected scipy.interpolate
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants