search

rapid7 / meterpreter

THIS REPO IS OBSOLETE. USE https://github.com/rapid7/metasploit-payloads INSTEAD
Other
325 stars 144 forks source link

getsystem_namedpipes_synch #182

Closed NickSampanis closed 8 years ago

NickSampanis commented 8 years ago

Instead of waiting with Sleep() for 500 ms. I implemented a semaphore which signals when the pipe has been created, as you have commented on todo. I tested the semaphores and they seem to work.

metasploit-public-bot commented 8 years ago

Can one of the admins verify this patch? For more information see: https://github.com/rapid7/metasploit-payloads/wiki/CI-Testing

kernelsmith commented 8 years ago

jenkins, this is ok to test

OJ commented 8 years ago

Thanks for this PR @NickSampanis, it looks good. Can we fallback on the Sleep() option if the creation of the semaphore fails? I'm sure this will never happen, but it'd be nice to know that a fallback is in place that worked in the past will allow things to work? Cheers!

NickSampanis commented 8 years ago

I told to @bcook-r7 at irc that there is a problem with alignment and he told that he will get it from here. The thing is that in my visual studio this code seems perfectly aligned :/ I could try fix it if you insist. About the semaphores, in my opinion if the creation of a semaphore fails. Something is going on with the memory allocation, so everything else might fail. Of course i/we could provide a simple if (hSem) before every semaphore function (and a else{sleep(500) in waitforsingleobject)

OJ commented 8 years ago

Thanks @NickSampanis, sorry I didn't see that conversation. I was just going through the "normal" review process :)

The reason it looks aligned in VS is because your tabs are set to 4 spaces.

I agree that allocation of the semaphore failing is an indication that things aren't good in general, but having a fallback in such scenarios would still be my preference. What do you think @bcook-r7 ?

bcook-r7 commented 8 years ago

I'd say that it's more likely that the whole process is close to dying if a semaphore cannot be allocated, and rather liked the change without the sleeps, TBH.