search

intel / isa-l_crypto

Other
271 stars 80 forks source link

Fix UB in sha1_mb_mgr_submit_sse_ni for win64 #117

Closed wildbook closed 1 year ago

wildbook commented 1 year ago

Avoid trashing r12 and r13 by storing xmm registers where they're intended to be stored.

I don't think these registers even have to be backed up, but I'll err on the side of caution and keep the functionality as-is since I'm not familiar with the codebase.

pablodelara commented 1 year ago

Thanks for this PR. r12 and r13 need to be preserved because they are used in sha1_ni_x2. I might do an alternative, and preserve r12 and r13 inside sha1_ni_x2 instead (looks more correct).

pablodelara commented 1 year ago

Apologies for the late response @wildbook. Actually, would you mind adding a comment that r12 and r13 are used in sha1_ni_x2, and we can go for your fix? Thanks1

wildbook commented 1 year ago

Sorry, I missed the notification for this @pablodelara!

I'll just comment the mov instructions then, feel free to move them around (or tell me to) if you'd rather have them somewhere else.

pablodelara commented 1 year ago

Now it's my turn to apologize, I also missed your message, @wildbook! Thanks for making the change! Could you squash those two commits into one and add the "signed-off" as per the contribution guidelines? Thanks!

pablodelara commented 1 year ago

@wildbook, will you have time to send an update? Otherwise, I will need to fix this with a separate commit. Thanks!

wildbook commented 1 year ago

@pablodelara Sorry for not replying earlier. I got busy and forgot about the PR, then missed the GitHub notification.

You're free to fix it however you want, including just copying my changes. I'm not going to be able to read up on and follow procedures right now, and I'd rather have it fixed than have you wait for me (more than you already have). For what it's worth I can confirm that all changes were made by me and that you're free to use them however you wish.

Thanks for understanding, and sorry once again for the wait!