diff options
author | Kinsey Moore <kmoore@digium.com> | 2014-04-30 13:08:07 +0000 |
---|---|---|
committer | Kinsey Moore <kmoore@digium.com> | 2014-04-30 13:08:07 +0000 |
commit | a7fc21783775008497b156505ace634dbbcc09b1 (patch) | |
tree | 4c8b4496198a0df6a351451d0bd6a66410eed97a /res/res_http_websocket.c | |
parent | 10f4d0f65dd8adcd33031e1bf87c9bcf8dd472d1 (diff) |
Websocket: Add session locking and delay close
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 websocket session writes
and by deferring session teardown until session destruction.
(closes issue ASTERISK-23605)
Review: https://reviewboard.asterisk.org/r/3481/
Reported by: Matt Jordan
........
Merged revisions 413123 from http://svn.asterisk.org/svn/asterisk/branches/11
........
Merged revisions 413124 from http://svn.asterisk.org/svn/asterisk/branches/12
git-svn-id: https://origsvn.digium.com/svn/asterisk/trunk@413125 65c4cc65-6c06-0410-ace0-fbb531ad65f3
Diffstat (limited to 'res/res_http_websocket.c')
-rw-r--r-- | res/res_http_websocket.c | 31 |
1 files changed, 23 insertions, 8 deletions
diff --git a/res/res_http_websocket.c b/res/res_http_websocket.c index 47fc7568b..d344340c7 100644 --- a/res/res_http_websocket.c +++ b/res/res_http_websocket.c @@ -79,6 +79,7 @@ struct ast_websocket { size_t reconstruct; /*!< Number of bytes before a reconstructed payload will be returned and a new one started */ unsigned int secure:1; /*!< Bit to indicate that the transport is secure */ unsigned int closing:1; /*!< Bit to indicate that the session is in the process of being closed */ + unsigned int close_sent:1; /*!< Bit to indicate that the session close opcode has been sent and no further data will be sent */ }; /*! \brief Structure definition for protocols */ @@ -164,6 +165,8 @@ static void session_destroy_fn(void *obj) { struct ast_websocket *session = obj; + ast_websocket_close(session, 0); + if (session->f) { fclose(session->f); ast_verb(2, "WebSocket connection from '%s' closed\n", ast_sockaddr_stringify(&session->address)); @@ -236,6 +239,11 @@ int AST_OPTIONAL_API_NAME(ast_websocket_server_remove_protocol)(struct ast_webso int AST_OPTIONAL_API_NAME(ast_websocket_close)(struct ast_websocket *session, uint16_t reason) { char frame[4] = { 0, }; /* The header is 2 bytes and the reason code takes up another 2 bytes */ + int res; + + if (session->close_sent) { + return 0; + } frame[0] = AST_WEBSOCKET_OPCODE_CLOSE | 0x80; frame[1] = 2; /* The reason code is always 2 bytes */ @@ -244,8 +252,12 @@ int AST_OPTIONAL_API_NAME(ast_websocket_close)(struct ast_websocket *session, ui put_unaligned_uint16(&frame[2], htons(reason ? reason : 1000)); session->closing = 1; + session->close_sent = 1; - return (fwrite(frame, 1, 4, session->f) == 4) ? 0 : -1; + ao2_lock(session); + res = (fwrite(frame, 1, 4, session->f) == 4) ? 0 : -1; + ao2_unlock(session); + return res; } @@ -281,14 +293,23 @@ int AST_OPTIONAL_API_NAME(ast_websocket_write)(struct ast_websocket *session, en put_unaligned_uint64(&frame[2], htonl(actual_length)); } + ao2_lock(session); + if (session->closing) { + ao2_unlock(session); + return -1; + } + if (fwrite(frame, 1, header_size, session->f) != header_size) { + ao2_unlock(session); return -1; } if (fwrite(payload, 1, actual_length, session->f) != actual_length) { + ao2_unlock(session); return -1; } fflush(session->f); + ao2_unlock(session); return 0; } @@ -531,13 +552,7 @@ int AST_OPTIONAL_API_NAME(ast_websocket_read)(struct ast_websocket *session, cha frame_size += (*payload_len); } - if (!session->closing) { - ast_websocket_close(session, 0); - } - - fclose(session->f); - session->f = NULL; - ast_verb(2, "WebSocket connection from '%s' closed\n", ast_sockaddr_stringify(&session->address)); + session->closing = 1; } else { ast_log(LOG_WARNING, "WebSocket unknown opcode %d\n", *opcode); /* We received an opcode that we don't understand, the RFC states that 1003 is for a type of data that can't be accepted... opcodes |