Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fixing memory allocation error in ngx_rtmp_live_join() #1781

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

Fahnenfluchtige
Copy link

The Svace static analysis tool identified a potential issue in the function ngx_rtmp_live_join(), where the return value of ngx_palloc() is used without NULL-checking (506-510):

if (ctx == NULL) {
        ctx = ngx_palloc(s->connection->pool, sizeof(ngx_rtmp_live_ctx_t)); // ngx_palloc may return NULL
        ngx_rtmp_set_ctx(s, ctx, ngx_rtmp_live_module);
    }

ngx_memzero(ctx, sizeof(*ctx)); 

ngx_palloc() is guaranteed not to return NULL only if there is enough space available in at least one of the existing memory pools in the linked list (see the implementation of ngx_palloc_small()). However, this guarantee is not absolute. If there is no available space, memory allocation will proceed by calling ngx_palloc_block(), which then invokes ngx_memalign() or posix_memalign()/memalign() to allocate memory. These system calls can return NULL if allocation fails. Therefore, it is always recommended to check the return value of ngx_palloc() and similar functions before using the allocated memory.

So it's necessary to add NULL-checking:

diff --git a/ngx_rtmp_live_module.c b/ngx_rtmp_live_module.c
index 5bebb9e..3826e8e 100644
--- a/ngx_rtmp_live_module.c
+++ b/ngx_rtmp_live_module.c
@@ -504,6 +504,19 @@ ngx_rtmp_live_join(ngx_rtmp_session_t *s, u_char *name, unsigned publisher)
 
     if (ctx == NULL) {
         ctx = ngx_palloc(s->connection->pool, sizeof(ngx_rtmp_live_ctx_t));
+        
+        if (ctx == NULL) {
+            ngx_log_error(NGX_LOG_ERR, s->connection->log, 0,
+                          "live: failed to allocate memory for context");
+
+            ngx_rtmp_send_status(s, "NetStream.Play.Failed", "error",
+                                 "Failed to allocate memory");
+
+            ngx_rtmp_finalize_session(s); 
+
+            return;
+        }
+
         ngx_rtmp_set_ctx(s, ctx, ngx_rtmp_live_module);
     }

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant