Re: [Qemu-devel] [PATCH v2 3/3] net: tap: restrict bridge name to IFNAMSIZ

[Daniel P . Berrangé]

On Mon, Jul 01, 2019 at 02:39:04PM +0530, P J P wrote:
> From: Prasad J Pandit <address@hidden>
> 
> The interface name in Linux interface request struct 'ifreq'
> OR in qemu-bridge-helper is defined to be of size IFNAMSIZ(=16),
> including the terminating null('\0') byte.
> 
> QEMU tap device, while invoking qemu-bridge-helper, supplies bridge
> name of 16 characters, restrict it to IFNAMESIZ-1 to accommodate
> terminating null('\0') byte.
> 
> Signed-off-by: Prasad J Pandit <address@hidden>
> ---
>  net/tap.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/net/tap.c b/net/tap.c
> index e8aadd8d4b..ca8536624c 100644
> --- a/net/tap.c
> +++ b/net/tap.c
> @@ -499,7 +499,7 @@ static int net_bridge_run_helper(const char *helper, 
> const char *bridge,
>      if (pid == 0) {
>          int open_max = sysconf(_SC_OPEN_MAX), i;
>          char fd_buf[6+10];
> -        char br_buf[6+IFNAMSIZ] = {0};
> +        char br_buf[5+IFNAMSIZ] = {0};
>          char helper_cmd[PATH_MAX + sizeof(fd_buf) + sizeof(br_buf) + 15];
>  
>          for (i = 3; i < open_max; i++) {

Playing games with multiple "perfectly" sized static buffers & snprintf
is madness. How about re-writing this method so that it just uses
g_strdup_printf() to dynamically format the helper_cmd string.

Alternatively we could get rid of the use of shell and directly exec
the helper program. This would let us just pass argv[] and avoid the
printf'ing entirely.

Regards,
Daniel
-- 
|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|