zsh-workers
 help / color / mirror / code / Atom feed
From: "VAN VLIERBERGHE Stef" <stef.van-vlierberghe@eurocontrol.int>
To: "Bart Schaefer" <schaefer@brasslantern.com>,
	<zsh-workers@zsh.org>,
	"Peter Stephenson" <p.w.stephenson@ntlworld.com>
Cc: "Godts, Jeroen" <godts@hp.com>,
	"Bart van den Heuvel" <bvandenh@redhat.com>,
	"Genot, Harry" <harry.genot@hp.com>,
	"WAROQUIERS Philippe" <philippe.waroquiers@eurocontrol.int>,
	"LORANG Geert" <geert.lorang@eurocontrol.int>
Subject: RE: uninitialized memory using a variable name of 31 or more characters
Date: Mon, 5 Dec 2011 21:17:36 +0100	[thread overview]
Message-ID: <1B2B2EF98D55CB41BD16F13B18B9B008134CC171@FFBRUE001.cfmu.corp.eurocontrol.int> (raw)
In-Reply-To: <111203132149.ZM30666@torch.brasslantern.com>

Thank you both.

Initially I thought it was safer to use the *bptr=0 fix because I was
not sure that the tokstr
buffer was always freshly allocated, if somewhere bptr is set back to
the beginning (tokstr)
then the problem would still exist (but be triggered even more rarely).

If you are sure that bptr can only advance, then it seems to me that you
can also remove
the temporary zero byte insertion further down in lex.c :

		    int sav = *bptr;
		    *bptr = '\0';
		    t = itype_end(t, IIDENT, 0);
		    if (t < bptr) {
			skipparens(Inbrack, Outbrack, &t);
		    } else {
			*bptr = sav;
		    }

I already asked Bart if RedHat would be willing to put in the valgrind
instrumentation needed
to catch errors like these systematically, and he was wondering if you
would agree to such a
change. I believe it would be a good idea, but I understand it would be
silly for RedHat to
spend the effort if you would not accept it.

Bart also confirmed that RedHat did run the test suite, so I guess they
expected the suite to
report the error.

Best regards.

-----Original Message-----
From: Bart Schaefer [mailto:schaefer@brasslantern.com]
Sent: Saturday 3 December 2011 22:22
To: zsh-workers@zsh.org
Cc: Godts, Jeroen; Bart van den Heuvel; Genot, Harry; VAN
VLIERBERGHE Stef
Subject: Re: uninitialized memory using a variable name of 31
or more characters

[Starting a new thread per Geoff's suggestion.]

On Dec 2, 10:54pm, VAN VLIERBERGHE Stef wrote:
} Subject: zsh-4.2.6-5.el5 rhel5.5 accesses uninitialized
memory in an assig
}
} A week ago I identified the problem [attached mail: lex.c
add() extends tokstr=calloc() by a non-zeroing hrealloc].
}
} The bug is (rarely) triggered by : AAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA=""

> ...
> A simple solution is to set *bptr=0 at the end of function
add, but I am not sure
> this has no other consequences, to be checked with zsh developers.

It's amazing to me that RedHat made this change without discovering that
it causes other/worse problems.  Zsh's own test suite fails if
that change
is made:

Test/A01grammar.ztst: starting.
ZTST_getsect:14: invalid subscript
[repeat for all other tests]

One problem, I suppose, is that this inability to run the tests
doesn't end
up causing the suite itself to report a failure:

**************************************
41 successful test scripts, 0 failures, 0 skipped
**************************************

} After:
} 	bptr = len + (tokstr = (char *)hrealloc(tokstr, bsiz, newbsiz));
} Add:
}       memset (bptr, 0, newbsiz - bsiz); /* len == bsiz, bptr
points at first re-allocated byte, newbsiz - bsiz is size added */

This seems to work fine, the full test suite runs and passes.

Index: Src/lex.c
--- ../zsh-forge/current/Src/lex.c      2011-09-19
08:26:12.000000000 -0700
+++ ./Src/lex.c 2011-12-03 08:59:39.000000000 -0800
@@ -583,6 +583,7 @@
            newbsiz = inbufct;

        bptr = len + (tokstr = (char *)hrealloc(tokstr, bsiz, newbsiz));
+       memset(bptr, 0, newbsiz - bsiz);  /* tokstr came from
calloc() */
        bsiz = newbsiz;
     }
 }

____
 
This message and any files transmitted with it are legally privileged and intended for the sole use of the individual(s) or entity to whom they are addressed. If you are not the intended recipient, please notify the sender by reply and delete the message and any attachments from your system. Any unauthorised use or disclosure of the content of this message is strictly prohibited and may be unlawful.
 
Nothing in this e-mail message amounts to a contractual or legal commitment on the part of EUROCONTROL, unless it is confirmed by appropriately signed hard copy.
 
Any views expressed in this message are those of the sender.


  reply	other threads:[~2011-12-05 20:17 UTC|newest]

Thread overview: 5+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
     [not found] <F8EF4D5D00775645B06F3DF4E25896D37E2972B582@GVW1118EXC.americas.hpqcorp.net>
     [not found] ` <A57EAE15146C184AA33F0DBEB2F830231D54E6D4@G4W3213.americas.hpqcorp.net>
2011-12-02 21:54   ` zsh-4.2.6-5.el5 rhel5.5 accesses uninitialized memory in an assignment statement " VAN VLIERBERGHE Stef
2011-12-03 17:13     ` Peter Stephenson
2011-12-03 21:21     ` uninitialized memory " Bart Schaefer
2011-12-05 20:17       ` VAN VLIERBERGHE Stef [this message]
2011-12-06  9:42         ` Peter Stephenson

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=1B2B2EF98D55CB41BD16F13B18B9B008134CC171@FFBRUE001.cfmu.corp.eurocontrol.int \
    --to=stef.van-vlierberghe@eurocontrol.int \
    --cc=bvandenh@redhat.com \
    --cc=geert.lorang@eurocontrol.int \
    --cc=godts@hp.com \
    --cc=harry.genot@hp.com \
    --cc=p.w.stephenson@ntlworld.com \
    --cc=philippe.waroquiers@eurocontrol.int \
    --cc=schaefer@brasslantern.com \
    --cc=zsh-workers@zsh.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.
Code repositories for project(s) associated with this public inbox

	https://git.vuxu.org/mirror/zsh/

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).