Segmentation fault at line 756 of ngx_http_ubus_module.c

[tobiaswaldvogel]

I'm using OpenWRT with nginx and luci_statistics on one of my devices and I am collecting data for quite a lot of other devices.

After adding another device the statistics overview page stopped working and I found messages in the syslog about signal 11 (SIGSEV) for nginx worker processes.
After compiling nginx with debug symbols and installing libc with debug symbols I was able to identify the origin with valgrind in line 756 of ngx_http_ubus_module.c, by the looks because in->buf->pos = 0.

After reading a little bit through the nginx API for ngx_http_read_client_request_body I figured out that this is related to client_body_buffer_size, which defaults on my device to 8192. If the request is bigger (in my case 11520) then it is stored in a temp file rather than in buffers.

From http://nginx.org/en/docs/dev/development_guide.html
"The body can be saved in memory buffers or file buffers, if the capacity specified by the client_body_buffer_size directive is not enough to fit the entire body in memory."

As a workaround I increased client_body_buffer_size to 128k, which solved the problem for me.
Probably it is not worth implementing the file read logic, as usually UBUS requests are pretty small. But I would suggest throwing an error message if in->buf->pos == 0, saying that client_body_buffer_size should be increased.

[Ansuel]

Yep I didn't implement the file logic stuff because normally you should never use ubus for very big request... wonder how uhttpd handles that... Can you check this?

[tobiaswaldvogel]

uhttpd seem to work in a complete different way, see code snippet below. As far as I have seen uh_ubus_data_send is called for each chunk of data, so it does not have this kind of problem.

Probably there are very few users running into this problem, so as said before for me the best and simplest solution would be just throwing and error message case in->buf->pos == 0 suggesting to increase client_body_buffer_size.

That would save to the time to identify the root cause for others

 912 static int uh_ubus_data_send(struct client *cl, const char *data, int len)
 913 {
 914         struct dispatch_ubus *du = &cl->dispatch.ubus;
 915 
 916         if (du->jsobj || !du->jstok)
 917                 goto error;
 918 
 919         du->post_len += len;
 920         if (du->post_len > UH_UBUS_MAX_POST_SIZE)
 921                 goto error;
 922 
 923         du->jsobj = json_tokener_parse_ex(du->jstok, data, len);
 924         return len;
 925 
 926 error:
 927         uh_ubus_single_error(cl, ERROR_PARSE);
 928         return 0;
 929 }
 930 
 931 static void uh_ubus_handle_request(struct client *cl, char *url, struct path_info *pi)
 932 {
 933         struct dispatch *d = &cl->dispatch;
 934         struct dispatch_ubus *du = &d->ubus;
 935         char *chr;
 936 
 937         du->url_path = strdup(url);
 938         if (!du->url_path) {
 939                 ops->client_error(cl, 500, "Internal Server Error", "Failed to allocate resources");
 940                 return;
 941         }
 942         chr = strchr(du->url_path, '?');
 943         if (chr)
 944                 chr[0] = '\0';
 945 
 946         du->legacy = false;
 947 
 948         switch (cl->request.method)
 949         {
 950         case UH_HTTP_MSG_GET:
 951                 uh_ubus_handle_get(cl);
 952                 break;
 953         case UH_HTTP_MSG_POST:
 954                 d->data_send = uh_ubus_data_send;
 955                 d->data_done = uh_ubus_handle_post;
 956                 d->close_fds = uh_ubus_close_fds;
 957                 d->free = uh_ubus_request_free;
 958                 du->jstok = json_tokener_new();
 959                 return;