[Xcb] [Bug 28526] New: sockname leaked in _xcb_get_auth_info

[Barton C Massey]

Uggh.  Yeah.

Attached is a patch that I haven't tested *at all* that
might still work and should in any case solve the memory
leak.  I *think* this addresses the need to know both the
socket info and the peer info except in the case of unix
domain sockets, where just the socket info is sufficient.

Could somebody try this with both local and remote
connections and verify that I haven't broken anything?

	Bart

In message <bug-28526-13485 at http.bugs.freedesktop.org/> you wrote:
> https://bugs.freedesktop.org/show_bug.cgi?id=28526
> 
>            Summary: sockname leaked in _xcb_get_auth_info
>            Product: XCB
>            Version: unspecified
>           Platform: Other
>         OS/Version: Solaris
>             Status: NEW
>           Severity: minor
>           Priority: medium
>          Component: Library
>         AssignedTo: xcb at lists.freedesktop.org
>         ReportedBy: alan.coopersmith at oracle.com
>          QAContact: xcb at lists.freedesktop.org
> 
> 
> Building from git commit 3f79628becbd3b0eff1aef804902eb739fac4403
> our leak checker found a memory leak of sockname in _xcb_get_auth_info()
> 
> Stepping through in the debugger I can see this happening:
> 
>   302       if ((sockname = get_peer_sock_name(getpeername, fd)) == NULL)
> 
> returns a newly allocated pointer for sockname, but doesn't set gotsockname.
> (gotsockname is only set in the == NULL case, which I don't understand.)
> 
> We then use that pointer to get the authptr:
> 
> (dbx) n
> stopped in _xcb_get_auth_info at line 314 in file "xcb_auth.c"
>   314       authptr = get_authptr(sockname, display);
> (dbx) n
> stopped in _xcb_get_auth_info at line 315 in file "xcb_auth.c"
>   315       if (authptr == 0)
> (dbx) n
> stopped in _xcb_get_auth_info at line 321 in file "xcb_auth.c"
>   321       info->namelen = memdup(&info->name, authptr->name,
> authptr->name_length);
> (dbx) n
> stopped in _xcb_get_auth_info at line 322 in file "xcb_auth.c"
>   322       if (!info->namelen)
> (dbx) print sockname
> sockname = 0x806c618
> (dbx) n
> stopped in _xcb_get_auth_info at line 325 in file "xcb_auth.c"
>   325       if (!gotsockname && (sockname = get_peer_sock_name(getsockname,
> fd)) == NULL)
> (dbx) print gotsockname
> gotsockname = 0
> (dbx) n      
> stopped in _xcb_get_auth_info at line 331 in file "xcb_auth.c"
>   331       ret = compute_auth(info, authptr, sockname);
> (dbx) print sockname
> sockname = 0x806e878
> 
> So at 325 we overwrote sockname, leaking the old pointer.
> 
> I don't quite understand the code here, so I'm not sure if the correct fix
> is setting gotsockname = 1 in a new else clause for the if clause at 302 or
> simply free(sockname) before overwriting it at 325.
> 
> This isn't a big leak - just 124 bytes when opening a connection to the server,
> which most programs only do once.
> 
> -- 
> Configure bugmail: https://bugs.freedesktop.org/userprefs.cgi?tab=email
> ------- You are receiving this mail because: -------
> You are the QA contact for the bug.
> You are the assignee for the bug.
> _______________________________________________
> Xcb mailing list
> Xcb at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/xcb