-
Notifications
You must be signed in to change notification settings - Fork 1.9k
ICP: Fix out of bounds write #9660
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
If gcm_mode_encrypt_contiguous_blocks() is called more than once in succession, with the accumulated lengths being less than blocksize, ctx->copy_to will be incorrectly advanced. Later, if out is NULL, the bcopy at line 114 will overflow ctx->gcm_copy_to since ctx->gcm_remainder_len is larger than the ctx->gcm_copy_to buffer can hold. The fix is to set ctx->copy_to only if it's not already set. For ZoL the issue may be academic, since in all my testing I wasn't able to hit neither of both conditions needed to trigger it, but other consumers can easily do so. Signed-off-by: Attila Fülöp <[email protected]>
Codecov Report
@@ Coverage Diff @@
## master #9660 +/- ##
==========================================
+ Coverage 79.32% 79.34% +0.01%
==========================================
Files 418 418
Lines 123544 123544
==========================================
+ Hits 98003 98026 +23
+ Misses 25541 25518 -23
Continue to review full report at Codecov.
|
With unmodified master |
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.
This change looks good to me. This does raise some interesting questions though about pushing ICP fixes back to Illumos.
Right, I already sent a mail to illumos-devel. Let's see if someone chimes in. |
Illumos issue 12048. |
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.
The ztest
appear to be unrelated, but I've resubmitted those builds to the CI for good measure. Thanks for following up with illumos so they can comment on and track the proposed fix.
Motivation and Context
Bug fix.
Description
If
gcm_mode_encrypt_contiguous_blocks()
is called more than once in succession, with the accumulated lengths being less thanblocksize
,ctx->copy_to
will be incorrectly advanced. Later, if out is NULL, bcopy() will overflowctx->gcm_copy_to
sincectx->gcm_remainder_len
is larger than thectx->gcm_copy_to
buffer can hold.For ZoL the issue may be academic, since in all my testing I wasn't able to hit neither of both conditions needed to trigger it, but other consumers can easily do so.
The fix is to set
ctx->copy_to
only if it's not already set.How Has This Been Tested?
As part of another branch this survived 6 hours of zloop. Currently zloop fails with unmodified master within one hour, so can't use zloop to test this PR, sorry.
Types of changes
Checklist:
Signed-off-by
.