ORT 1.28.3 Cherry Picks - #32806
Open
adrastogi wants to merge 10 commits into
Open
ORT 1.28.3 Cherry Picks#32806adrastogi wants to merge 10 commits into
adrastogi wants to merge 10 commits into
Conversation
… scalar beta (#32610) ### Description `EmbedLayerNormalizationShapeInference` validated the `beta` input's rank using the wrong shape: `beta_dims` was aliased to `gamma_shape.dim()` instead of `beta_shape.dim()`. As a result, a rank-1 `gamma` would let the rank check pass even when `beta` is a 0-dim scalar, and execution reached `beta_shape.dim(0).dim_value()` — an unchecked protobuf repeated-field access on an empty `dim` list, causing an out-of-bounds read during session load, before provider/kernel assignment. ### Fix - Alias `beta_dims` to `beta_shape.dim()` so the rank check guards the correct input; a scalar `beta` is now rejected via `fail_shape_inference` instead of reaching the OOB access. This matches the adjacent `gamma` validation. - Correct the `gamma` error message ("2 dimension" → "1 dimension") to reflect the actual check (cherry picked from commit 7054657)
### Description <!-- Describe your changes. --> Add an explicit validation in `Node::UpdateInputArgCount()` to reject a node that supplies actual inputs while its bound operator/function schema declares no formal input parameters. ### Motivation and Context <!-- - Why is this change required? What problem does it solve? - If it fixes an open issue, please link to the issue here. --> When `op.inputs()` is empty but the node has ≥1 input, the arg-count adjustment loop was skipped yet the trailing `input_arg_count.push_back(arg_count_left)` still ran unconditionally, producing `InputArgCount().size() == 1` against `op.inputs().size() == 0`. This size-invariant violation later caused an out-of-bounds read at `op.inputs()[i]` in `InferAndVerifyTypeMatch` during `Graph::Resolve()`. This is reachable only via a model-local function in a custom domain: for registered ops, `OpSchema::Verify` in the ONNX checker rejects the extra inputs first, but for custom-domain function calls the checker performs no arity validation. (cherry picked from commit 613bc03)
…te finalization (#32641) ### Description `SessionState::FinalizeSessionStateImpl` iterates over the subgraphs attached to each node and unconditionally downcasts the node's kernel to `controlflow::IControlFlowKernel`, then calls `SetupSubgraphExecutionInfo` on it: ```cpp // Downcast is safe, since only control flow nodes have subgraphs auto& control_flow_kernel = static_cast<controlflow::IControlFlowKernel&>(*p_op_kernel); ``` The "only control flow nodes have subgraphs" invariant is not enforced anywhere. Node::Init materializes a subgraph for any attribute of type GRAPH, with no schema gate, so an ordinary (non-control-flow) node can carry a subgraph. When it does, the downcast above is invalid: IControlFlowKernel adds a vtable slot that a plain OpKernel does not have, so the call reads an out-of-bounds vtable slot — an undefined-behavior type confusion that crashes (observed as an access violation inside FinalizeSessionStateImpl). This is reachable by loading a crafted/malformed model whose non-control-flow node has a GRAPH-typed attribute (for example, an op that permits unchecked attributes). ORT should reject such a model with a clear error instead of executing the bad cast. ### Fix Gate the downcast on a virtual predicate: - Add `OpKernel::IsControlFlowKernel()` returning `false` by default. - Override it to `true` on `IControlFlowKernel`, which covers If/Loop/Scan and their CUDA derivatives. - Check it in `FinalizeSessionStateImpl` before the cast and return an error otherwise. A virtual predicate is used rather than `dynamic_cast` because `onnxruntime_DISABLE_RTTI` is on by default. There is no behavior change for valid models: only the control-flow kernels inherit `IControlFlowKernel`, and they always return `true`. Adds `InferenceSessionTests.SubgraphAttributeOnNonControlFlowNodeIsRejected` test case, which builds a model whose non-control-flow node carries a `GRAPH` attribute and asserts that session initialization fails gracefully instead of triggering the downcast. Existing If/Loop/Scan subgraph tests cover the no-regression path. (cherry picked from commit b95cf78)
…full-byte tensor (#32611) ### Description The allocation planner's `SameSize()` decided buffer reuse by comparing the C++ carrier size of the element type (`elt_type->Size()`) plus the logical shape. Packed sub-byte types (`int4`/`uint4`) have the same 1-byte carrier size as `int8`/`uint8`, but one carrier stores 2 logical elements. As a result a `uint4[N]` tensor (physical `ceil(N/2)` bytes) and a `uint8[N]` tensor (physical `N` bytes) were treated as the same size, and the smaller `uint4` buffer was reused for the `uint8` output. Writing the `uint8` tensor into that half-sized buffer overflows it. ### Fix - `SameSize()` now also requires the sub-element packing density (`GetNumSubElems()`) to match, so tensors are only considered the same size when their physical storage bytes are actually equal. - `ExecutionFrame::AllocateMLValueTensorPreAllocateBuffer()` adds a defense-in-depth capacity check that rejects a reuse when the requested tensor needs more storage bytes than the buffer being reused (the previous check only compared logical element counts). - Added `AllocationPlannerTest.AvoidReuseOfPackedSubByteBufferForFullByteTensor`, which builds `X(float) → uint4 → float → uint8 → float` so a dead `uint4[1024]` output would be reused by a `uint8[1024]` output. Without the fix the planner reuses the buffer (`alloc_kind == kReuse`) and the output is corrupted; with the fix the test passes and the model runs correctly (cherry picked from commit 061eecb)
…izer (#32609) ### Problem `OuterScopeNodeArgLocationAccumulator` (session_state.cc) maps a Loop/Scan node's explicit inputs onto the subgraph's inputs using `GraphViewer::GetInputs()`, which *excludes* initializer-backed inputs. When a body declares an input that is also a body initializer, `GetInputs()` is shorter than the parent node's input list, so indexing `subgraph_inputs[arg_idx]` reads past the end of the vector and dereferences an invalid `NodeArg` pointer during session initialization. This causes a crash / heap out-of-bounds read (observable under ASan) when loading a malformed model containing such a Loop/Scan. ### Fix In the Loop/Scan≥9 branch of `OuterScopeNodeArgLocationAccumulator`: - If `subgraph.GetInputs().size()` does not equal the parent node's input count, return an `INVALID_GRAPH` status naming the parent node and both counts, instead of indexing. When the size difference is explained by initializer-backed inputs (`GetInputsIncludingInitializers()` is larger), the message adds a hint pointing at that as the cause. - Add `Loop.BodyInputAlsoInitializerIsRejected` test: builds a Loop whose body input `state_in` is also a body initializer, and asserts `InferenceSession::Initialize()` fails with `INVALID_GRAPH` and the expected message — i.e. rejected cleanly rather than crashing. The test asserts on the status code and message text on purpose, so that it keeps covering this path rather than silently falling through to the `ORT_ENFORCE` downstream. (cherry picked from commit 09dfa6a)
### Summary Strengthens attribute/input validation in three contrib-op shape-inference functions in bert_defs.cc so malformed or malicious models are rejected at `Graph::Resolve` instead of triggering out-of-bounds reads or signed-integer overflow during shape inference. ### Changes `CausalConvWithState`: validate the ndim attribute is in [1, 3] and cross-check tensor ranks against it (weight == ndim+2, channels-first input == ndim+2, channels-last input >= 3) before the spatial-dim loop indexes input.dim(2+i). Previously only a rank >= 2 guard existed, so ndim=2/3 with a low-rank input read past the shape's dimensions. `GatedDeltaNet`: require the head counts/sizes to be positive and add step-by-step overflow guards before computing the state_update capsule width, preventing signed int64 overflow (UB) in state_update_capacity * (num_heads_v + num_heads_k*head_size_qk + num_heads_v*head_size_v). `GroupQueryAttention` / `SparseAttention`: check the parsed `total_sequence_length` initializer is non-empty before indexing data[0]. Backport notes for rel-1.28.3: - GatedDeltaNet does not exist on this branch, so its overflow guards and RejectsStateUpdateWidthOverflow test are dropped. - CausalConvWithState on this branch predates channels_last/state_window/ dilation. Only the ndim range check and the weight/input rank == ndim + 2 checks are backported; ChannelsLastInputRankBelowThreeIsRejected is dropped. - GroupQueryAttention on this branch has no sliding_window_cache, so the call site only gains the total_sequence_length_index argument. (cherry picked from commit bca9d15)
### Description Add an explicit rank-consistency check in `convTransposeWithDynamicPadsShapeInference()` so that a derived `kernel_shape` (built from the weight tensor's rank when the `kernel_shape` attribute is absent) must have the same length as `n_input_dims` (derived from the input tensor's rank) before it is used, exactly like the existing check on the explicit-attribute path. ### Motivation and Context `n_input_dims` is computed from input `X`'s rank. When `kernel_shape` is not given as an attribute, it is instead derived from weight `W`'s rank. If `W`'s rank disagrees with `X`'s rank, the resulting `kernel_shape` (and `effective_kernel_shape`, sized identically) is shorter or longer than `n_input_dims`. Both directions are unsafe: - **Shorter** (e.g. X rank 5, W rank 3): the output-shape loop iterates `n_input_dims` times over `effective_kernel_shape`/`pads`, reading past the end of the shorter vector. - **Longer** (e.g. X rank 3, W rank 5): the dilation loop iterates `kernel_shape.size()` times over `dilations`, which is always sized to `n_input_dims`, reading past the end of `dilations`. This runs during `Graph::Resolve()` (model load), before any kernel `Compute()` validation is ever reached. ### Testing Added `ConvTransposeWithDynamicPads_MismatchedInputWeightRank` (X rank 5, W rank 3, no `kernel_shape` attribute), asserting the model now fails cleanly at the existing kernel-level rank check (`"X num_dims does not match W num_dims."`) instead of hitting the shape-inference OOB read during load. (cherry picked from commit 2ecddca)
### Description `InferenceContextImpl::getInputData` (graph.cc) indexed `node_.InputDefs()` directly by the schema input index during shape inference, with no bounds check. Add the same bounds check already used by `DataPropagationContextImpl::getInputData` in the same file, so `getInputData` returns `nullptr` (treated as "input not available") instead of indexing out of bounds. ### Motivation and Context ONNX schemas can declare trailing inputs as optional (e.g. `ConvTransposeWithDynamicPads`'s `Pads`), so a node can validly omit them entirely, shrinking `Node::InputDefs()` below the schema's full input count. A `TypeAndShapeInferenceFunction` that queries such an omitted optional input's data (e.g. via `ctx.getInputData(index)`) on such a node reads past the end of the vector, causing an out-of-bounds read. (cherry picked from commit e5f272c)
### Description
`build_full_ort` in `.github/workflows/linux_minimal_build.yml` now
installs `uuid-dev` before building:
```yaml
# This job builds with --use_coreml. The coremltools modelpackage sources include <uuid/uuid.h>,
# which is provided by uuid-dev on Ubuntu.
- name: Install libuuid development files
run: sudo apt-get update -y && sudo apt-get install -y uuid-dev
```
- Scoped to `build_full_ort` only — it is the sole job in this workflow
that builds with `--use_coreml` directly on the runner; the rest build
inside the CI docker image or don't enable CoreML.
- Placed before `Build Full ORT and Prepare Test Files`, so the
header/library are present both when CMake configures and when
`ModelPackage.cpp` compiles.
- No CMake change: `cmake/onnxruntime_providers_coreml.cmake` already
does `find_path`/`find_library` for libuuid on Linux (with an actionable
`FATAL_ERROR`) and links it into `onnxruntime_providers_coreml`.
### Motivation and Context
`Linux CPU Minimal Build E2E` / `build_full_ort` fails in [run
34290674202, job
102276280773](https://github.com/microsoft/onnxruntime/actions/runs/34290674202):
```
_deps/coremltools-src/modelpackage/src/ModelPackage.cpp:35:10: fatal error: uuid/uuid.h: No such file or directory
```
The job enables the CoreML EP, and the fetched coremltools modelpackage
sources include `<uuid/uuid.h>`, supplied on Ubuntu by `uuid-dev`.
Neither the workflow nor the reusable `setup-build-tools` action (which
only provisions cmake/ccache/vcpkg) installs apt packages, so the header
is only available when vcpkg's `coreml-ep` feature happens to provide
it; without it Ninja stops while compiling
`onnxruntime_providers_coreml`.
---------
Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com>
Co-authored-by: tianleiwu <30328909+tianleiwu@users.noreply.github.com>
(cherry picked from commit f26e546)
### Description Raise the Android minimal baseline threshold by 3 KiB, from 1,440,768 to 1,443,840 bytes. The security fixes cherry-picked for 1.28.3 add validation to code that is compiled into the minimal build (session state finalization, execution frame, Loop/Scan subgraph handling). The deterministic section total grows from 1,440,538 bytes (rel-1.28.3 base, run 36072293072) to 1,443,098 bytes (+2,560 bytes), exceeding the old threshold by 2,330 bytes. The new threshold leaves 742 bytes of headroom. This is a release-branch-only change; main tracks its own baseline (see #32772). Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
50 tasks
This branch has not been deployed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This cherry-picks the following commits for the release:
CI fixes:
uuid-devfor the Linux minimal-build CoreML job; that check also fails on therel-1.28.3base)All picks except #32607 applied cleanly with
git cherry-pick -x, in the order they merged into main.#32607 required a manual backport because
bert_defs.cchas diverged on main since 1.28 branched:GroupQueryAttention/SparseAttention: backported as-is (total_sequence_length_indexof 6 / 7, plus the single-element check). The GQA call site in 1.28 has nosliding_window_cache, so it only gets the new argument.CausalConvWithState: the 1.28 schema predateschannels_last,state_window, anddilation. Only thendimin [1, 3] check and theweight/inputrank ==ndim + 2checks are backported. These match what the 1.28 CPU kernel already enforces.ChannelsLastInputRankBelowThreeIsRejectedis dropped.GatedDeltaNet: this op does not exist on 1.28.3, so its hunk and test are dropped.Validation (Windows x64, CPU EP, RelWithDebInfo):
onnxruntime_test_all: 1847 tests, 0 failures.onnxruntime_provider_test: 5370 tests, 0 failures.