From mboxrd@z Thu Jan 1 00:00:00 1970 X-Spam-Checker-Version: SpamAssassin 3.4.4 (2020-01-24) on inbox.vuxu.org X-Spam-Level: X-Spam-Status: No, score=-3.4 required=5.0 tests=DKIM_SIGNED,DKIM_VALID, DKIM_VALID_AU,MAILING_LIST_MULTI,RCVD_IN_DNSWL_MED,UNPARSEABLE_RELAY autolearn=ham autolearn_force=no version=3.4.4 Received: (qmail 17934 invoked from network); 18 Aug 2021 19:28:25 -0000 Received: from zero.zsh.org (2a02:898:31:0:48:4558:7a:7368) by inbox.vuxu.org with ESMTPUTF8; 18 Aug 2021 19:28:25 -0000 ARC-Seal: i=1; cv=none; a=rsa-sha256; d=zsh.org; s=rsa-20210803; t=1629314905; b=C/Qqj/bm3yzcl/TsnOCZqZOHcZde8LpuNBJB8ets/SWZvmtdAgDSwvRpBPA9LUJjtk/smzgyNj MUSJKw+5jYNNIjm7wOLCyN5YfBdEjlYSloYDqTFqZ4evXPnIGEXMd+7luNE5G/TdaX+VXrI8Xn U5CSIr8yEiH/6Mthv/RQa8qKMd5lqec9av37XmrQTd6qsTiiez0BCtAsYQMaxSCTHW5FHNyX1t WizGeCuy/Xj0YqveLOqm035izQn08qL60Vr0Ty1qtYRU/HtSV3KE+abgn5DG8E1LQdGg8/MXCu zgYrPw9a/jfzHtW+8i9kTLG1Fj85zEaS65dSjbaBqGMp4A==; ARC-Authentication-Results: i=1; zsh.org; iprev=pass (smtpq2.tb.ukmail.iss.as9143.net) smtp.remote-ip=212.54.57.97; dkim=pass header.d=ntlworld.com header.s=meg.feb2017 header.a=rsa-sha256; dmarc=pass header.from=ntlworld.com; arc=none ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed; d=zsh.org; s=rsa-20210803; t=1629314905; bh=BMT0Gb0Wvf/eeBha0tbuljwqFirlfUA/uGfWs7k4ncU=; h=List-Archive:List-Owner:List-Post:List-Unsubscribe:List-Subscribe:List-Help: List-Id:Sender:Content-Transfer-Encoding:MIME-Version:Content-Type: References:In-Reply-To:Date:To:From:Subject:Message-ID:DKIM-Signature: DKIM-Signature; b=ivsQ4OlCeDAltRE73gtqZXBOTO3wgJSnJOVwcwvruiaMIDYyCoAro6SKTlA8ACE0nVjRZjAJzM jTGqSeLdjlXhJaUscgWOLYcVmgfBsaclGYRdItKwb19Mg2B5TTQXImlrGqsT+D0QQRhrCw3Mqz +ksaaqMH/bi0QuO74GqnyRZvHQaLDARPTWlBfLyIJgD0Jrem+TRfjzhVd3nfPHmlt5LXr+m27G VWGmCMlLR9I3UM7A8e0E64DpTkhg6AJkTcJRmv8EvSQ+CEKAx70yu/AlMWDBs0+m5M4FO4OrDz S3UNydINLqSbVJwnqWDZgyORNB21ObAR7k+OYHHwz6mtZQ==; DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=zsh.org; s=rsa-20210803; h=List-Archive:List-Owner:List-Post:List-Unsubscribe: List-Subscribe:List-Help:List-Id:Sender:Content-Transfer-Encoding: Mime-Version:Content-Type:References:In-Reply-To:Date:To:From:Subject: Message-ID:Reply-To:Cc:Content-ID:Content-Description:Resent-Date:Resent-From :Resent-Sender:Resent-To:Resent-Cc:Resent-Message-ID; bh=X6uDWFH0Ltse59sbEvVCz0fZDG7dUvm8ALV98Ywwfts=; b=g4QezInTZYk2zOuX/jyPTcJh4E WfKGz6ANxjeLdTsEMoc2NdYnTCuLRECY61kabPJv78lTaAVNSm1ks/r55XZymSCVt9L6FIMFEYu2R z6tBfVmOzOP5a+LsDHCs7oAAD3KqbAjt87++uaJftgCvi8DD4XO6yEyKolSR4T8h1H9HzJu9Gka0Q jty1kUDWAwwdKHJw+4P/dy7ji+QRa76IX+815a84TDcodVpvB1iA5Ig9SOcPNBlSlKIytpDJ9Vxi8 t/j22QQ/z5GnQuLaUEgg8MN/wp7auUKGNz3KBLw/kA1jE1F/SkuXNEDq7taM1JMQq8NqRAHsbnOHi JFzmhlUA==; Received: from authenticated user by zero.zsh.org with local id 1mGREd-000Mgy-PI; Wed, 18 Aug 2021 19:28:23 +0000 Authentication-Results: zsh.org; iprev=pass (smtpq2.tb.ukmail.iss.as9143.net) smtp.remote-ip=212.54.57.97; dkim=pass header.d=ntlworld.com header.s=meg.feb2017 header.a=rsa-sha256; dmarc=pass header.from=ntlworld.com; arc=none Received: from smtpq2.tb.ukmail.iss.as9143.net ([212.54.57.97]:60810) by zero.zsh.org with esmtps (TLS1.2:ECDHE-RSA-AES128-GCM-SHA256:128) id 1mGRE4-000MN5-EJ; Wed, 18 Aug 2021 19:27:49 +0000 Received: from [212.54.57.107] (helo=csmtp3.tb.ukmail.iss.as9143.net) by smtpq2.tb.ukmail.iss.as9143.net with esmtp (Exim 4.86_2) (envelope-from ) id 1mGRE4-0004x9-2X for zsh-workers@zsh.org; Wed, 18 Aug 2021 21:27:48 +0200 Received: from pws-Zeus ([86.7.189.51]) by cmsmtp with ESMTPA id GRE3mtaTLbkrTGRE3m4xr0; Wed, 18 Aug 2021 21:27:47 +0200 X-Originating-IP: [86.7.189.51] X-Authenticated-Sender: p.w.stephenson@ntlworld.com X-Spam: 0 X-Authority: v=2.4 cv=OYD7sjfY c=1 sm=1 tr=0 ts=611d5f33 cx=a_exe a=mDzK/eG20+r+ucIvzJc7BQ==:117 a=mDzK/eG20+r+ucIvzJc7BQ==:17 a=IkcTkHD0fZMA:10 a=MhDmnRu9jo8A:10 a=6ZuCY_HIQIYlpt_9tuwA:9 a=QEXdDO2ut3YA:10 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=ntlworld.com; s=meg.feb2017; t=1629314867; bh=X6uDWFH0Ltse59sbEvVCz0fZDG7dUvm8ALV98Ywwfts=; h=Subject:From:To:Date:In-Reply-To:References; b=llIh6wDpHFY3S1uh3JFqmKQjkhpE9+4xPUnuaxu+S9FHYlj494qowEeSSKTJNJs61 1RdFYHaKFnFuV+8AjJ65lCK7pOveLzUS1F5sz/SNYWZw2DTQhm7vNvFp1hnhdvXKvt yJtzm11rfr+BR5viOC/TJjPtpA0g9X2kf2aPyw2fKqNXIFq0K0VDeIMGWGW12pJvBC gv3kp55Pt2pfc3fZWMwVz2p0bor+XloUwfy0xQcZC4OxUVsZpSnDIEQtNm6Xvxha1Z Df4vhXHHKxhB1khapxprJ/vpxXoV/76LRJuH4CF6Wi6wYaACSG7KQFfnmuL+CifEAw BmMplZmoDvxHw== Message-ID: <5509351db61721486dc9d27c7d816e8bea23b829.camel@ntlworld.com> Subject: Re: [BUG] malloc inside signal handler From: Peter Stephenson To: zsh-workers@zsh.org Date: Wed, 18 Aug 2021 20:27:47 +0100 In-Reply-To: <1888954006.948395.1629188495083@mail2.virginmedia.com> References: <20210724225048.ly7qpdvi55j3h5mi@localhost> <20210731202547.yxzvwevxvy22ywm3@localhost> <1888954006.948395.1629188495083@mail2.virginmedia.com> Content-Type: text/plain; charset="UTF-8" X-Mailer: Evolution 3.28.5-0ubuntu0.18.04.2 Mime-Version: 1.0 Content-Transfer-Encoding: 7bit X-CMAE-Envelope: MS4xfFrZhfGR5lGor77yIO/HwsnICPZcP3sI3wYvO4W8tJNUI4NasrdTUOZM/J21st2a6FQOOi6lShNW7DCguuz4WVHqSqx5WzuVzmVSEKbGe9V/jvGqmrLE Oqmd6M9U75OpIXdXxA7wsRW8oLXGtZCwQW3YQVFM7H2Cb41T3FqOOiYUjHa4PjVCLZqJ1/s+u7v1Pw== X-Seq: 49290 Archived-At: X-Loop: zsh-workers@zsh.org Errors-To: zsh-workers-owner@zsh.org Precedence: list Precedence: bulk Sender: zsh-workers-request@zsh.org X-no-archive: yes List-Id: List-Help: List-Subscribe: List-Unsubscribe: List-Post: List-Owner: List-Archive: On Tue, 2021-08-17 at 09:21 +0100, Peter Stephenson wrote: > Avoiding stdio for input of this sort doesn't actually look infeasible, > either --- mentions of bshin (buffered shell input from files) are > restricted to only a couple of files. But it's kind of annoying as this > is exactly the job stdio is designed for. As I noted, this isn't that difficult, and this should remove the problem --- all the memory management is now in places where we would expect it. I don't know what the general feeling is about this, but as a piece of long-term robustness and with no immediately upcoming release I'd incline to getting this or something like it in for trying out. It could usefully do with a scan for leaks, which is the most likely thing I can see going wrong. Obviously I've checked myself, but potentially there are possible leaks of file descriptors, stack entries, and allocated buffers, so it's not completely trivial. In case it's not clear: the allocation of the buffer is not tied to the file descriptor in use. The only time we need to allocate a new buffer other than at initialisation is when we're temporarily saving the existing input while we source a different file. Otherwise, we simply need to reset the buffer pointers. pws diff --git a/Src/init.c b/Src/init.c index 3d6c94d04..878a53a37 100644 --- a/Src/init.c +++ b/Src/init.c @@ -1229,7 +1229,7 @@ setupshin(char *runscript) /* * Finish setting up SHIN and its relatives. */ - bshin = SHIN ? fdopen(SHIN, "r") : stdin; + shinbufalloc(); if (isset(SHINSTDIN) && !SHIN && unset(INTERACTIVE)) { #ifdef _IONBF setvbuf(stdin, NULL, _IONBF, 0); @@ -1384,9 +1384,9 @@ init_misc(char *cmd, char *zsh_name) dosetopt(RESTRICTED, 1, 0, opts); if (cmd) { if (SHIN >= 10) - fclose(bshin); + close(SHIN); SHIN = movefd(open("/dev/null", O_RDONLY | O_NOCTTY)); - bshin = fdopen(SHIN, "r"); + shinbufreset(); execstring(cmd, 0, 1, "cmdarg"); stopmsg = 1; zexit((exit_pending || shell_exiting) ? exit_val : lastval, ZEXIT_NORMAL); @@ -1409,7 +1409,6 @@ source(char *s) int tempfd = -1, fd, cj; zlong oldlineno; int oldshst, osubsh, oloops; - FILE *obshin; char *old_scriptname = scriptname, *us; char *old_scriptfilename = scriptfilename; unsigned char *ocs; @@ -1426,7 +1425,6 @@ source(char *s) /* save the current shell state */ fd = SHIN; /* store the shell input fd */ - obshin = bshin; /* store file handle for buffered shell input */ osubsh = subsh; /* store whether we are in a subshell */ cj = thisjob; /* store our current job number */ oldlineno = lineno; /* store our current lineno */ @@ -1439,7 +1437,7 @@ source(char *s) if (!prog) { SHIN = tempfd; - bshin = fdopen(SHIN, "r"); + shinbufsave(); } subsh = 0; lineno = 1; @@ -1507,10 +1505,10 @@ source(char *s) if (prog) freeeprog(prog); else { - fclose(bshin); + close(SHIN); fdtable[SHIN] = FDT_UNUSED; SHIN = fd; /* the shell input fd */ - bshin = obshin; /* file handle for buffered shell input */ + shinbufrestore(); } subsh = osubsh; /* whether we are in a subshell */ thisjob = cj; /* current job number */ diff --git a/Src/input.c b/Src/input.c index f568cc135..3c6ad7002 100644 --- a/Src/input.c +++ b/Src/input.c @@ -80,11 +80,6 @@ /**/ int SHIN; -/* buffered shell input for non-interactive shells */ - -/**/ -FILE *bshin; - /* != 0 means we are reading input from a string */ /**/ @@ -129,7 +124,116 @@ static struct instacks *instack, *instacktop; static int instacksz = INSTACK_INITIAL; -/* Read a line from bshin. Convert tokens and * +/* Size of buffer for non-interactive command input */ + +#define SHINBUFSIZE 8192 + +/* Input buffer for non-interactive command input */ +static char *shinbuffer; + +/* Pointer into shinbuffer */ +static char *shinbufptr; + +/* End of contents read into shinbuffer */ +static char *shinbufendptr; + +/* Entry on SHIN buffer save stack */ +struct shinsaveentry { + /* Next entry on stack */ + struct shinsaveentry *next; + /* Saved shinbuffer */ + char *buffer; + /* Saved shinbufptr */ + char *ptr; + /* Saved shinbufendptr */ + char *endptr; +}; + +/* SHIN buffer save stack */ +struct shinsaveentry *shinsavestack; + +/* Reset the input buffer for SHIN, discarding any pending input */ + +/**/ +void +shinbufreset(void) +{ + shinbufendptr = shinbufptr = shinbuffer; +} + +/* Allocate a new shinbuffer + * + * Only called at shell initialisation and when saving on the stack. + */ + +/**/ +void +shinbufalloc(void) +{ + shinbuffer = zalloc(SHINBUFSIZE); + shinbufreset(); +} + +/* Save entry on SHIN buffer save stack */ + +/**/ +void +shinbufsave(void) +{ + struct shinsaveentry *entry = + (struct shinsaveentry *)zalloc(sizeof(struct shinsaveentry)); + + entry->next = shinsavestack; + entry->buffer = shinbuffer; + entry->ptr = shinbufptr; + entry->endptr = shinbufendptr; + + shinsavestack = entry; + + shinbufalloc(); +} + +/* Restore entry from SHIN buffer save stack */ + +/**/ +void +shinbufrestore(void) +{ + struct shinsaveentry *entry = shinsavestack; + + zfree(shinbuffer, SHINBUFSIZE); + + shinbuffer = entry->buffer; + shinbufptr = entry->ptr; + shinbufendptr = entry->endptr; + + shinsavestack = entry->next; + zfree(entry, sizeof(struct shinsaveentry)); +} + +/* Get a character from SHIN, -1 if none available */ + +/**/ +static int +shingetchar(void) +{ + int nread; + + if (shinbufptr < shinbufendptr) + return STOUC(*shinbufptr++); + + shinbufreset(); + do { + errno = 0; + nread = read(SHIN, shinbuffer, SHINBUFSIZE); + } while (nread < 0 && errno == EINTR); + if (nread <= 0) + return -1; + shinbufendptr = shinbuffer + nread; + return STOUC(*shinbufptr++); +} + +/* Read a line from SHIN. Convert tokens and * * null characters to Meta c^32 character pairs. */ /**/ @@ -147,11 +251,7 @@ shingetline(void) winch_unblock(); dont_queue_signals(); for (;;) { - /* Can't fgets() here because we need to accept '\0' bytes */ - do { - errno = 0; - c = fgetc(bshin); - } while (c < 0 && errno == EINTR); + c = shingetchar(); if (c < 0 || c == '\n') { winch_block(); restore_queue_signals(q);