Re: [Xen-devel] [Patch v2 3/4] tools/xenstored: Avoid buffer overflows while setting up sockets

[Andrew Cooper <andrew.cooper3@xxxxxxxxxx>]

Ping?  This v2 patch appears to have slipped through the cracks from my
set of Coverity fixes.

~Andrew

On 25/11/13 14:38, Andrew Cooper wrote:
> Coverity ID: 1055996 1056002
>
> Cache the xs_daemon_socket{,_ro}() strings to save pointlessly
> re-snprintf()'ing the same path, and add explicit size checks against
> addr.sun_path before strcpy()'ing into it.
>
> Signed-off-by: Andrew Cooper <andrew.cooper3@xxxxxxxxxx>
> CC: Ian Campbell <Ian.Campbell@xxxxxxxxxx>
> CC: Ian Jackson <Ian.Jackson@xxxxxxxxxxxxx>
> CC: Matthew Daley <mattd@xxxxxxxxxxx>
>
> ---
> Changes in v2:
>  * Use logic similar to f220279c14
> ---
>  tools/xenstore/xenstored_core.c |   27 ++++++++++++++++++---------
>  1 file changed, 18 insertions(+), 9 deletions(-)
>
> diff --git a/tools/xenstore/xenstored_core.c b/tools/xenstore/xenstored_core.c
> index ccfdaa3..2324e53 100644
> --- a/tools/xenstore/xenstored_core.c
> +++ b/tools/xenstore/xenstored_core.c
> @@ -1718,6 +1718,9 @@ static void init_sockets(int **psock, int **pro_sock)
>  {
>       struct sockaddr_un addr;
>       int *sock, *ro_sock;
> +     const char *soc_str = xs_daemon_socket();
> +     const char *soc_str_ro = xs_daemon_socket_ro();
> +
>       /* Create sockets for them to listen to. */
>       *psock = sock = talloc(talloc_autofree_context(), int);
>       *sock = socket(PF_UNIX, SOCK_STREAM, 0);
> @@ -1731,19 +1734,25 @@ static void init_sockets(int **psock, int **pro_sock)
>       talloc_set_destructor(ro_sock, destroy_fd);
>  
>       /* FIXME: Be more sophisticated, don't mug running daemon. */
> -     unlink(xs_daemon_socket());
> -     unlink(xs_daemon_socket_ro());
> +     unlink(soc_str);
> +     unlink(soc_str_ro);
>  
>       addr.sun_family = AF_UNIX;
> -     strcpy(addr.sun_path, xs_daemon_socket());
> +
> +     if(strlen(soc_str) >= sizeof(addr.sun_path))
> +             barf_perror("socket string '%s' too long", soc_str);
> +     strcpy(addr.sun_path, soc_str);
>       if (bind(*sock, (struct sockaddr *)&addr, sizeof(addr)) != 0)
> -             barf_perror("Could not bind socket to %s", xs_daemon_socket());
> -     strcpy(addr.sun_path, xs_daemon_socket_ro());
> +             barf_perror("Could not bind socket to %s", soc_str);
> +
> +     if(strlen(soc_str_ro) >= sizeof(addr.sun_path))
> +             barf_perror("socket string '%s' too long", soc_str_ro);
> +     strcpy(addr.sun_path, soc_str_ro);
>       if (bind(*ro_sock, (struct sockaddr *)&addr, sizeof(addr)) != 0)
> -             barf_perror("Could not bind socket to %s",
> -                         xs_daemon_socket_ro());
> -     if (chmod(xs_daemon_socket(), 0600) != 0
> -         || chmod(xs_daemon_socket_ro(), 0660) != 0)
> +             barf_perror("Could not bind socket to %s", soc_str_ro);
> +
> +     if (chmod(soc_str, 0600) != 0
> +         || chmod(soc_str_ro, 0660) != 0)
>               barf_perror("Could not chmod sockets");
>  
>       if (listen(*sock, 1) != 0


_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
http://lists.xen.org/xen-devel