[release/10.0-rc1] [Android] Normalize RSA no-padding output to modulus size to fix VerifyHash #118918
Conversation
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
…lysis (#118769) A couple times in the past I needed a way to say "if X is not part of the program, eliminate the entire basic block". We can do this for allocated types (this is how branches under `is` checks elimination works), but we can't do this for more general "characteristics". This introduces a mechanism where AOT compiler and CoreLib (or System.Private.* universe in general) can define whole program tags such as "whole program has X in it" and CoreLib can condition code on the presence of this tag. This is easier shown than described, so I extracted the first use of this into a separate commit. In this commit, we eliminate code that tries looking for `StackTraceHiddenAttribute` if we know the whole program has no `StackTraceHiddenAttribute` in it. With this code eliminated, #118640 can then eliminate all custom attributes on methods, which in turn plays into #118718 and we can eliminate enum boxing even when StackTraceSupport is not set to false (right now #118718 really needs the StackTraceSupport=false to get rid of boxed enums; we get more boxed enums from method attributes). We have a new node that represents the characteristic. The node can be dropped into the graph wherever needed. ILScanner then uses this to condition parts of the method body on this characteristic node. We need similar logic in the substitution IL provider because we need to guarantee that RyuJIT is not going to see basic blocks we didn't scan. So we treat it as a substitution during codegen phase too.
Contributing towards [#115479](#115479) Contains implementations for the set of [SVE2 FP APIs](#94018 (comment)), aside from `ConvertToSingleOdd` and `ConvertToSingleOddRoundToOdd`. This is due to concerns I have about the API proposal for these intrinsics being incorrect, when I have a resolution to [my comments](#94018 (comment)) on this they will be implemented.
The assertion only ended up printing stderr, but it would be useful to have all the information about the command that was run (the exit code, stdout).
and if the GCGen0MaxBudget config is specified, always honor it including for DATAS
With large block counts, the CFG convergence takes a long time and performs a lot of Meets, allocating a new Dictionary for each Value in each block. This adds up to gigabytes of allocations in the process. Ideally we would fix the performance issue with some optimization, but I'm still working on a proper solution. To fix the issue for .NET 10 in the meantime, we can add a limit to the number of iterations to bail out if we reach an excessively large number of iterations. The largest number of iterations in the runtime build is 2600, so 10,000 seems fairly reasonable. The build time of the project in issue #118121 is around ~20 seconds with 10,000 iterations and ~70 seconds with 20,000.
Mop-up commits from the end of the merge window for RC1
…8829) This lets us keep some of the constant-time indexing advantages of the IList iterator, without the GVM overhead of Select. There is a small size increase here, but nowhere near the cost of the GVM. In a pathological generated example for GVMs the cost was: 1. .NET 9: 12 MB 2. .NET 10 w/out this change: 2.2 MB 3. .NET 10 w/ this change: 2.3 MB In a real-world example (AzureMCP), the size attributed to System.Linq was: 1. .NET 9: 1.2 MB 2. .NET 10 w/out this change: 340 KB 3. .NET 10 w/ this change: 430 KB This seems like a good tradeoff. We mostly keep the algorithmic complexity the same across the size/speed-opt versions, and just tradeoff on the margins. We could probably continue to improve this in the future.
simonrozsival
added
area-System.Security
Servicing-consider
jeffschwMSFT
approved these changes
Aug 20, 2025
Member
jeffschwMSFT
left a comment
jeffschwMSFT
left a comment
There was a problem hiding this comment.
lgtm. we will take for consideration in 10 rc1
bartonjs
approved these changes
Aug 20, 2025
simonrozsival
requested review from
MichalStrehovsky,
marek-safar and
sbomer
as code owners
simonrozsival
changed the title
dotnet-policy-service
bot
added
the
linkable-framework
Member
|
@jeffschwMSFT @bartonjs we agreed to move this to RC2 instead of RC1 so I will close this PR now and open a new one. |
simonrozsival
changed the title
Merged
4 tasks
This was referenced Aug 21, 2025
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
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.
Backport of #118880 to release/10.0-rc1
/cc @simonrozsival
Customer Impact
Certain cryptographic operations (
RSA.VerifyData,RSA.VerifyHash) do not work properly on some devices, for example on Samsung phones running Android 15. I am not aware of any report from a customer, but this might be because this issue can be hard to detect, as it only affects some versions of Android.Regression
Testing
Manual testing - existing unit tests are now passing on the affected Android devices and emulators (specific OS versions of certain vendors). The issue was missed because it did not reproduce consistently on the android emulators used in our Helix queue. It appears that this is a change in behavior in the platform library caused by an OS update.
Risk
Low. The fix modifies crypto code but the changes are scoped just to Android and they are well tested through unit tests.
/cc @bartonjs @vcsjones @vitek-karas