Project

General

Profile

Actions

Bug #2962

closed

SUN_LEN in sock_addr.c (1.4.53, 1.4.54)

Added by lighthouse2 almost 5 years ago. Updated almost 5 years ago.

Status:
Fixed
Priority:
Normal
Category:
core
Target version:
ASK QUESTIONS IN Forums:

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
Actions #1

Updated by gstrauss almost 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'.

Actions #2

Updated by gstrauss almost 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

Actions #3

Updated by gstrauss almost 5 years ago

  • Status changed from Patch Pending to Fixed
  • % Done changed from 0 to 100
Actions

Also available in: Atom