Bug #2962
closedSUN_LEN in sock_addr.c (1.4.53, 1.4.54)
Description
Hello.
Valgrind has discovered a problem:
==14321== Memcheck, a memory error detector ==14321== Copyright (C) 2002-2017, and GNU GPL'd, by Julian Seward et al. ==14321== Using Valgrind-3.13.0 and LibVEX; rerun with -h for copyright info ==14321== Command: ./lighttpd -D -f ./running.conf -m ./.libs ==14321== ==14321== Syscall param socketcall.connect(serv_addr.sun_path) points to unaddressable byte(s) ==14321== at 0x584C8B4: connect (connect.c:26) ==14321== by 0x12BB3D: gw_spawn_connection (gw_backend.c:478) ==14321== by 0x12F6F7: gw_set_defaults_backend (gw_backend.c:1516) ==14321== by 0x6744830: mod_fastcgi_set_defaults (mod_fastcgi.c:80) ==14321== by 0x132DE5: plugins_call_set_defaults (plugin.c:384) ==14321== by 0x113A3F: server_main (server.c:1471) ==14321== by 0x1124D1: main (server.c:2098) ==14321== Address 0x5d72f3e is 0 bytes after a block of size 30 alloc'd ==14321== at 0x4C2FB0F: malloc (in /usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so) ==14321== by 0x12CCEC: gw_proc_sockaddr_init.isra.22 (gw_backend.c:412) ==14321== by 0x12F6D2: gw_set_defaults_backend (gw_backend.c:1510) ==14321== by 0x6744830: mod_fastcgi_set_defaults (mod_fastcgi.c:80) ==14321== by 0x132DE5: plugins_call_set_defaults (plugin.c:384) ==14321== by 0x113A3F: server_main (server.c:1471) ==14321== by 0x1124D1: main (server.c:2098) ==14321== ==14321== Syscall param socketcall.bind(my_addr.sun_path) points to unaddressable byte(s) ==14321== at 0x584C877: bind (syscall-template.S:78) ==14321== by 0x12BC4D: gw_spawn_connection (gw_backend.c:513) ==14321== by 0x12F6F7: gw_set_defaults_backend (gw_backend.c:1516) ==14321== by 0x6744830: mod_fastcgi_set_defaults (mod_fastcgi.c:80) ==14321== by 0x132DE5: plugins_call_set_defaults (plugin.c:384) ==14321== by 0x113A3F: server_main (server.c:1471) ==14321== by 0x1124D1: main (server.c:2098) ==14321== Address 0x5d72f3e is 0 bytes after a block of size 30 alloc'd ==14321== at 0x4C2FB0F: malloc (in /usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so) ==14321== by 0x12CCEC: gw_proc_sockaddr_init.isra.22 (gw_backend.c:412) ==14321== by 0x12F6D2: gw_set_defaults_backend (gw_backend.c:1510) ==14321== by 0x6744830: mod_fastcgi_set_defaults (mod_fastcgi.c:80) ==14321== by 0x132DE5: plugins_call_set_defaults (plugin.c:384) ==14321== by 0x113A3F: server_main (server.c:1471) ==14321== by 0x1124D1: main (server.c:2098) ==14321== ^C==14321== ==14321== HEAP SUMMARY: ==14321== in use at exit: 1,918 bytes in 7 blocks ==14321== total heap usage: 1,974 allocs, 1,967 frees, 138,582 bytes allocated ==14321== ==14321== LEAK SUMMARY: ==14321== definitely lost: 0 bytes in 0 blocks ==14321== indirectly lost: 0 bytes in 0 blocks ==14321== possibly lost: 0 bytes in 0 blocks ==14321== still reachable: 1,918 bytes in 7 blocks ==14321== suppressed: 0 bytes in 0 blocks ==14321== Rerun with --leak-check=full to see details of leaked memory ==14321== ==14321== For counts of detected and suppressed errors, rerun with: -v ==14321== ERROR SUMMARY: 2 errors from 2 contexts (suppressed: 0 from 0)
It looks as the reason in calculating the length of the sockaddr_un (sock_addr.c - 1.4.54):
memcpy(saddr->un.sun_path, str, hostlen); #if defined(SUN_LEN) *len = SUN_LEN(&saddr->un); #else /* stevens says: */ *len = hostlen + sizeof(saddr->un.sun_family); #endif
When the sockets library provides a sockaddr_un structure to the application, the sun_len field is set to SUN_LEN(&sa)+1, where sa is the name of the sockaddr_un variable. This length thus includes the null byte which terminates the file name. https://www.ibm.com/support/knowledgecenter/en/SSB27U_7.1.0/com.ibm.zvm.v710.kiml0/sktunix.htm
config file:
server.modules = ( "mod_rewrite", "mod_access", "mod_indexfile", "mod_fastcgi", "mod_cgi", "mod_alias", "mod_openssl", "mod_accesslog" ) server.document-root = "/tmp" index-file.names = ( "index.ws" ) mimetype.assign = ( ".pdf" => "application/pdf", ".sig" => "application/pgp-signature", ".spl" => "application/futuresplash", ".class" => "application/octet-stream", ".ps" => "application/postscript", ".torrent" => "application/x-bittorrent", ".dvi" => "application/x-dvi", ".gz" => "application/x-gzip", ".pac" => "application/x-ns-proxy-autoconfig", ".swf" => "application/x-shockwave-flash", ".tar.gz" => "application/x-tgz", ".tgz" => "application/x-tgz", ".tar" => "application/x-tar", ".zip" => "application/zip", ".mp3" => "audio/mpeg", ".m3u" => "audio/x-mpegurl", ".wma" => "audio/x-ms-wma", ".wax" => "audio/x-ms-wax", ".ogg" => "application/ogg", ".wav" => "audio/x-wav", ".gif" => "image/gif", ".jar" => "application/x-java-archive", ".jpg" => "image/jpeg", ".jpeg" => "image/jpeg", ".png" => "image/png", ".xbm" => "image/x-xbitmap", ".xpm" => "image/x-xpixmap", ".xwd" => "image/x-xwindowdump", ".css" => "text/css", ".html" => "text/html", ".htm" => "text/html", ".js" => "text/javascript", ".asc" => "text/plain", ".c" => "text/plain", ".cpp" => "text/plain", ".log" => "text/plain", ".conf" => "text/plain", ".text" => "text/plain", ".txt" => "text/plain", ".dtd" => "text/xml", ".xml" => "text/xml", ".mpeg" => "video/mpeg", ".mpg" => "video/mpeg", ".mov" => "video/quicktime", ".qt" => "video/quicktime", ".avi" => "video/x-msvideo", ".asf" => "video/x-ms-asf", ".asx" => "video/x-ms-asf", ".wmv" => "video/x-ms-wmv", ".bz2" => "application/x-bzip", ".tbz" => "application/x-bzip-compressed-tar", ".tar.bz2" => "application/x-bzip-compressed-tar", # default mime type "" => "application/octet-stream", ) accesslog.filename = "/tmp/access.log" server.errorlog = "/tmp/error.log" url.access-deny = ( "~", ".inc" ) static-file.exclude-extensions = ( ".lua" ) server.pid-file = "/tmp/lighttpd.pid" fastcgi.server = ( ".ws" => ( "localhost" => ( "socket" => "/tmp/kepler-fastcgi.socket", "bin-path" => "/usr/bin/wsapi.fcgi", "max-procs" => 1 ) ) ) fastcgi.debug = 0
Updated by gstrauss over 5 years ago
Thanks for the report.
Looks like the origin of the SUN_LEN code in lighttpd is commit b9b8a46f
I'll have to test with SUN_LEN(&sa)+1. If the OS is ignoring the length provided and instead recalculating it, then adding the '\0' to the length might be the right answer. Alternatively, we might consider allocating an extra byte in gw_backend.c and setting it to '\0'.
Updated by gstrauss over 5 years ago
- Category set to core
- Status changed from New to Patch Pending
- Target version changed from 1.4.x to 1.4.55
I checked what getsockname() returned for addrlen and it included the '\0' in the length, so I changed the code to do the same with SUN_LEN()+1
Updated by gstrauss over 5 years ago
- Status changed from Patch Pending to Fixed
- % Done changed from 0 to 100
Applied in changeset 186ce8a2b105cce1c8b5133f40b0b429e5547105.
Also available in: Atom