[asterisk-dev] [Code Review] 3481: Websocket: Add locking around session access and modification

[Matt Jordan]

-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviewboard.asterisk.org/r/3481/#review11756
-----------------------------------------------------------



branches/11/res/res_http_websocket.c
<https://reviewboard.asterisk.org/r/3481/#comment21562>

    So, while locking may solve the issue, there's something more insidious about this part of the code that doesn't sit well with me. (Note: this is the actual culprit that causes a crash in the websocket write)
    
    I'm not sure that the way this is currently handled is the right way to handle a AST_WEBSOCKET_OPCODE_CLOSE. The session destructor will already close the the file descriptor. Ideally, we'd just let the destruction of the session do this work for us.
    
    It feels like the right way to handle this may be to just let the caller of ast_websocket_read know that they were told that the session needs to die. That would let them de-ref the session appropriately. If a concurrent write was occurring at the same time, when the write completes, the session would be terminated.
    
    Now, whether or not it's allowed to have a write complete when you've just been told to close the websocket is another question. If not, then we have to keep all of the locking in here.


- Matt Jordan


On April 25, 2014, 1:46 p.m., opticron wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviewboard.asterisk.org/r/3481/
> -----------------------------------------------------------
> 
> (Updated April 25, 2014, 1:46 p.m.)
> 
> 
> Review request for Asterisk Developers.
> 
> 
> Bugs: ASTERISK-23605
>     https://issues.asterisk.org/jira/browse/ASTERISK-23605
> 
> 
> Repository: Asterisk
> 
> 
> Description
> -------
> 
> This resolves a race condition where data could be written to a NULL FILE pointer causing a crash as a websocket connection was in the process of shutting down by adding locking to accesses and modifications of the websocket session struct.
> 
> 
> Diffs
> -----
> 
>   branches/11/res/res_http_websocket.c 413007 
> 
> Diff: https://reviewboard.asterisk.org/r/3481/diff/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> opticron
> 
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.digium.com/pipermail/asterisk-dev/attachments/20140425/68b6b93b/attachment.html>