-
Notifications
You must be signed in to change notification settings - Fork 584
fix(pd): change numel function return type from int to size_t to prevent overflow #4924
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
Conversation
…ent overflow Co-authored-by: njzjz <9496702+njzjz@users.noreply.github.com>
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.
Pull Request Overview
This PR fixes an integer overflow vulnerability in the Paddle backend's numel function by changing the return type and calculations from int to size_t. The function computes tensor element counts by multiplying dimensions, which can overflow with large tensors (e.g., 25 billion elements).
- Changed
numelfunction return type frominttosize_twith safe casting - Updated all calling sites to use
size_tvariables instead ofint - Removed the TODO comment acknowledging the overflow issue
Reviewed Changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
| source/api_cc/include/DeepPotPD.h | Modified numel function signature and implementation to use size_t with explicit casting |
| source/api_cc/src/DeepPotPD.cc | Updated all variables that store numel results to use size_t type |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## devel #4924 +/- ##
=======================================
Coverage 84.29% 84.29%
=======================================
Files 703 703
Lines 68728 68728
Branches 3573 3572 -1
=======================================
Hits 57935 57935
Misses 9653 9653
Partials 1140 1140 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
HydrogenSulfate
left a comment
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.
size_t is more appropriate than int: https://github.com/PaddlePaddle/Paddle/blob/53f2a48fd48d0f97eef53a0a88c1b79283b39b88/paddle/fluid/inference/api/paddle_api.h#L121%E2%80%9D
…ent overflow (deepmodeling#4924) The `numel` function in the Paddle backend was using `int` for computing tensor element counts, which can overflow for large tensors. This fix changes the return type and intermediate calculations to `size_t` to handle larger tensor sizes safely. ## Problem The original implementation multiplied tensor dimensions as `int` values: ```cpp int numel(const paddle_infer::Tensor& x) const { // TODO: There might be a overflow problem here for multiply int numbers. int ret = 1; std::vector<int> x_shape = x.shape(); for (std::size_t i = 0, n = x_shape.size(); i < n; ++i) { ret *= x_shape[i]; // Can overflow for large tensors } return ret; } ``` For large tensors (e.g., shape `[50000, 50000, 10]` = 25 billion elements), this causes integer overflow and returns negative values. ## Solution - Changed return type from `int` to `size_t` - Changed intermediate calculations to use `size_t` with explicit casting - Updated all calling sites to use `size_t` variables - Removed the TODO comment since the overflow issue is now resolved ```cpp size_t numel(const paddle_infer::Tensor& x) const { size_t ret = 1; std::vector<int> x_shape = x.shape(); for (std::size_t i = 0, n = x_shape.size(); i < n; ++i) { ret *= static_cast<size_t>(x_shape[i]); // Safe from overflow } return ret; } ``` The `size_t` type can handle up to 2^64 elements on 64-bit systems (vs 2^31 for `int`), making it appropriate for tensor element counts. This change is backward compatible since `std::vector::resize()` and other consumers already accept `size_t`. Fixes deepmodeling#4551. <!-- START COPILOT CODING AGENT TIPS --> --- ✨ Let Copilot coding agent [set things up for you](https://github.com/deepmodeling/deepmd-kit/issues/new?title=✨+Set+up+Copilot+instructions&body=Configure%20instructions%20for%20this%20repository%20as%20documented%20in%20%5BBest%20practices%20for%20Copilot%20coding%20agent%20in%20your%20repository%5D%28https://gh.io/copilot-coding-agent-tips%29%2E%0A%0A%3COnboard%20this%20repo%3E&assignees=copilot) — coding agent works faster and does higher quality work when set up for your repo. --------- Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com> Co-authored-by: njzjz <9496702+njzjz@users.noreply.github.com>
The
numelfunction in the Paddle backend was usingintfor computing tensor element counts, which can overflow for large tensors. This fix changes the return type and intermediate calculations tosize_tto handle larger tensor sizes safely.Problem
The original implementation multiplied tensor dimensions as
intvalues:For large tensors (e.g., shape
[50000, 50000, 10]= 25 billion elements), this causes integer overflow and returns negative values.Solution
inttosize_tsize_twith explicit castingsize_tvariablesThe
size_ttype can handle up to 2^64 elements on 64-bit systems (vs 2^31 forint), making it appropriate for tensor element counts. This change is backward compatible sincestd::vector::resize()and other consumers already acceptsize_t.Fixes #4551.
✨ Let Copilot coding agent set things up for you — coding agent works faster and does higher quality work when set up for your repo.