Re: [Qemu-devel] [PATCH for 2.10] slirp/smb: Replace constant strings by glib string

[Eric Blake]

On 04/07/2017 09:32 AM, Dr. David Alan Gilbert (git) wrote:
> From: "Dr. David Alan Gilbert" <address@hidden>
> 
> gcc 7 (on fedora 26) objects to many of the snprintf's
> in the smb path and command creation because it can't
> figure out that the smb_dir (i.e. the /tmp dir for the configuration)
> is known to be short.
> 
> Replace all these fixed length buffers by g_str* functions that dynamically
> allocate and use g_dir_make_tmp to make the directory.
> (It's fairly new glib but we have a compat function for it).
> 
> Signed-off-by: Dr. David Alan Gilbert <address@hidden>
> ---
>  net/slirp.c | 30 +++++++++++++++++-------------
>  1 file changed, 17 insertions(+), 13 deletions(-)
> 
> diff --git a/net/slirp.c b/net/slirp.c
> index f97ec23345..9f6521190b 100644
> --- a/net/slirp.c
> +++ b/net/slirp.c
> @@ -80,7 +80,7 @@ typedef struct SlirpState {
>      Slirp *slirp;
>      Notifier exit_notifier;
>  #ifndef _WIN32
> -    char smb_dir[128];
> +    gchar *smb_dir;

Does it really have to be gchar? That's one of the more pointless
typedefs in glib; and I think 'char *' is just fine.


>  
> -    snprintf(smb_cmdline, sizeof(smb_cmdline), "%s -l %s -s %s",
> +    smb_cmdline = g_strdup_printf("%s -l %s -s %s",
>               CONFIG_SMBD_COMMAND, s->smb_dir, smb_conf);

Gross that we're parsing a command line through a shell, but
pre-existing and looks like CONFIG_SMBD_COMMAND, s->smb_dir, and
smb_conf should all be enough under our control to ensure that there are
no shell metacharacters and therefore exploits possible.

The cleanup is useful, and resolves one of the build issues I pointed
out earlier on Rawhide (looks like it is now Fedora 26 in addition to
Rawhide that have new-enough gcc).  In that thread, we argued that it's
not going to be essential to get this in for 2.9, but as more and more
people move to newer gcc, it will probably be a candidate for
qemu-stable for 2.9.1 in addition to 2.10.

Reviewed-by: Eric Blake <address@hidden>

-- 
Eric Blake, Principal Software Engineer
Red Hat, Inc.           +1-919-301-3266
Virtualization:  qemu.org | libvirt.org

signature.asc
Description: OpenPGP digital signature