netfilter-devel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Eric Dumazet <eric.dumazet@gmail.com>
To: pablo@netfilter.org
Cc: netfilter-devel@vger.kernel.org, netfilter@vger.kernel.org
Subject: Re: [PATCH] iptables-restore: move code to add_param_to_argv, cleanup (fix gcc-4.7)
Date: Mon, 23 Jul 2012 13:29:06 +0200	[thread overview]
Message-ID: <1343042946.2626.10727.camel@edumazet-glaptop> (raw)
In-Reply-To: <1343039903-7230-1-git-send-email-pablo@netfilter.org>

On Mon, 2012-07-23 at 12:38 +0200, pablo@netfilter.org wrote:
> From: Pablo Neira Ayuso <pablo@netfilter.org>
> 
> This patch seems to be a mere cleanup that moves the parameter parsing
> code to add_param_to_argv.
> 
> But, in reality, it also fixes iptables whe compiled with gcc-4.7.
> 
> Moving param_buffer declaration out of the loop seems to resolve the
> issue. gcc-4.7 seems to be generating bad code regarding param_buffer.
> 
> @@ -380,9 +380,9 @@
>                         quote_open = 0;
>                         escaped = 0;
>                         param_len = 0;
> +                       char param_buffer[1024];
> 
>                         for (curchar = parsestart; *curchar; curchar++) {
> -                               char param_buffer[1024];
> 
>                                 if (quote_open) {
>                                         if (escaped) {
> 
> But I have hard time to apply this patch in such a way. Instead, I came
> up with the idea of this cleanup, which does not harm after all (and fixes
> the issue for us).
> 
> Sorry, I didn't have the time to further debug this issue, but it would be
> worth to investigate what's going wrong and ping gcc people.

Bug seems that iptables forgot that "char param_buffer[1024];" can
disappear at the end of the block :

for (curchar = parsestart; *curchar; curchar++) {
	char param_buffer[1024];
	...
}

// here param_buffer[1024] is lost, so any var pointing 
// to it can mess stack

previous gcc were probably not so aggressive.




  reply	other threads:[~2012-07-23 11:29 UTC|newest]

Thread overview: 10+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2012-07-23 10:38 [PATCH] iptables-restore: move code to add_param_to_argv, cleanup (fix gcc-4.7) pablo
2012-07-23 11:29 ` Eric Dumazet [this message]
2012-07-23 11:38   ` Eric Dumazet
2012-07-23 13:08     ` Pablo Neira Ayuso
2012-07-30  1:37     ` Pablo Neira Ayuso
2012-07-30  1:40       ` Pablo Neira Ayuso
2012-08-02 19:37       ` Jan Engelhardt
2012-08-03  9:15         ` Pablo Neira Ayuso
2012-07-23 12:19 ` Jan Engelhardt
2012-07-23 13:04   ` Pablo Neira Ayuso

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=1343042946.2626.10727.camel@edumazet-glaptop \
    --to=eric.dumazet@gmail.com \
    --cc=netfilter-devel@vger.kernel.org \
    --cc=netfilter@vger.kernel.org \
    --cc=pablo@netfilter.org \
    /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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).