Open Bug 571699 Opened 14 years ago Updated 1 year ago
Multiple Locking problems in ssl3
_Compress MACEncrypt Record & ssl3 _Handle Record
User-Agent: Mozilla/5.0 (Windows; U; Windows NT 6.1; en-US) AppleWebKit/533.4 (KHTML, like Gecko) Chrome/5.0.375.55 Safari/533.4 Build Identifier: 1. Because both of these functions modify data in the read or write spec, they need to acquire the spec write lock. However, both functions only acquire the spec read lock instead. 2. ssl3_HandleRecord releases the spec lock too early; immediately after releasing the lock, it continues to use the crSpec in order to decompress the data. The release of the lock needs to done afterwards. 3. If decompression fails, then ssl3_CompressMACEncryptRecord returns without releasing the spec lock. Reproducible: Always
Comment on attachment 450883 [details] [diff] [review] Proposed fixes. Thanks for the patch. There are several patches for various different problems presently awaiting review (or recently reviewed) that all touch this same code (especially the label spec_locked_loser). They must all be reconciled. I'll review this with the others.
Attachment #450883 - Attachment is obsolete: false
When I moved the release down in ssl3_HandleRecord, I missed an error condition in which I needed to add lock release. This patch fixes that new problem and the pre-existing ones.
Assignee: nobody → brian
Version: unspecified → trunk
Comment on attachment 450898 [details] [diff] [review] Fix another case where a lock needs to be released Brian, thanks for your patches. Whenever you submit an NSS patch, please request review for it. (Let me know if you cannot.) For patches to libSSL, please ask me for review. Likewise for lib/freebl. For patches to lib/softken and lib/pk11wrap, ask rrelyea. For libpkix, ask Alexei.volkov. For other parts, you can ask any of us.
Attachment #450898 - Flags: review?(nelson)
Nelson, thanks for the information. Almost all of my pending patches conflict with this one. If this one can get committed or rejected then I can submit patches for all the bugs I have marked as a dependency of this bug. If there's some reason that this patch can't get committed before the other bugs need to be fixed, then I will rewrite those other patches to use the spec read lock instead.
No longer blocks: 571795
No longer blocks: 571732
What is missing for this bug, a review flag?
Assignee: brian → nobody
You need to log in before you can comment on or make changes to this bug.