All of lore.kernel.org
 help / color / mirror / Atom feed
From: "Daniel P. Berrangé" <berrange@redhat.com>
To: P J P <ppandit@redhat.com>
Cc: Riccardo Schirone <rschiron@redhat.com>,
	Jason Wang <jasowang@redhat.com>, Li Qiang <liq3ea@gmail.com>,
	Qemu Developers <qemu-devel@nongnu.org>,
	Prasad J Pandit <pjp@fedoraproject.org>
Subject: Re: [Qemu-devel] [PATCH v2 3/3] net: tap: restrict bridge name to IFNAMSIZ
Date: Mon, 1 Jul 2019 10:37:49 +0100	[thread overview]
Message-ID: <20190701093749.GE3573@redhat.com> (raw)
In-Reply-To: <20190701090904.31312-4-ppandit@redhat.com>

On Mon, Jul 01, 2019 at 02:39:04PM +0530, P J P wrote:
> From: Prasad J Pandit <pjp@fedoraproject.org>
> 
> 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 <pjp@fedoraproject.org>
> ---
>  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 :|


  reply	other threads:[~2019-07-01  9:41 UTC|newest]

Thread overview: 11+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-07-01  9:09 [Qemu-devel] [PATCH v2 0/3] restrict bridge interface name to IFNAMSIZ P J P
2019-07-01  9:09 ` [Qemu-devel] [PATCH v2 1/3] qemu-bridge-helper: restrict " P J P
2019-07-01  9:43   ` Daniel P. Berrangé
2019-07-01  9:58     ` P J P
2019-07-01  9:09 ` [Qemu-devel] [PATCH v2 2/3] qemu-bridge-helper: move repeating code in parse_acl_file P J P
2019-07-01  9:44   ` Daniel P. Berrangé
2019-07-01  9:09 ` [Qemu-devel] [PATCH v2 3/3] net: tap: restrict bridge name to IFNAMSIZ P J P
2019-07-01  9:37   ` Daniel P. Berrangé [this message]
2019-07-01  9:57     ` P J P
2019-07-01  9:21 ` [Qemu-devel] [PATCH v2 0/3] restrict bridge interface " no-reply
2019-07-01 10:08 ` no-reply

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20190701093749.GE3573@redhat.com \
    --to=berrange@redhat.com \
    --cc=jasowang@redhat.com \
    --cc=liq3ea@gmail.com \
    --cc=pjp@fedoraproject.org \
    --cc=ppandit@redhat.com \
    --cc=qemu-devel@nongnu.org \
    --cc=rschiron@redhat.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.