From owner-freebsd-hackers Fri Jul 26 22:12:40 2002 Delivered-To: freebsd-hackers@freebsd.org Received: from mx1.FreeBSD.org (mx1.FreeBSD.org [216.136.204.125]) by hub.freebsd.org (Postfix) with ESMTP id EA4AA37B400 for ; Fri, 26 Jul 2002 22:12:32 -0700 (PDT) Received: from smtp.noos.fr (aragon.noos.net [212.198.2.75]) by mx1.FreeBSD.org (Postfix) with ESMTP id 9AA6643E67 for ; Fri, 26 Jul 2002 22:12:31 -0700 (PDT) (envelope-from cyrille.lefevre@laposte.net) Received: (qmail 12273945 invoked by uid 0); 27 Jul 2002 05:12:29 -0000 Received: from unknown (HELO gits.gits.dyndns.org) ([212.198.229.153]) (envelope-sender ) by 212.198.2.75 (qmail-ldap-1.03) with SMTP for ; 27 Jul 2002 05:12:29 -0000 Received: from gits.gits.fr.invalid (b6ag9ney7802uqui@localhost [127.0.0.1]) by gits.gits.dyndns.org (8.12.5/8.12.5) with ESMTP id g6R5CS5P008982 for ; Sat, 27 Jul 2002 07:12:29 +0200 (CEST) (envelope-from cyrille.lefevre@laposte.net) Date: Sat, 27 Jul 2002 07:12:24 +0200 To: Tony Finch Cc: freebsd-hackers@FreeBSD.ORG, freebsd-ports@FreeBSD.ORG Subject: Re: sed -i has difficulty with read-only files Message-ID: <20020727051224.GA7996@gits.dyndns.org> References: <20020726165021.C7551@chiark.greenend.org.uk> Mime-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20020726165021.C7551@chiark.greenend.org.uk> User-Agent: Mutt/1.3.99i Organization: ACME X-Face: V|+c;4!|B?E%BE^{E6);aI.[< Mail-Followup-To: cyrille.lefevre+dated+1028178747.705301@laposte.net, dot@dotat.at, freebsd-hackers@FreeBSD.ORG, freebsd-ports@FreeBSD.ORG X-Delivery-Agent: TMDA/0.58 Sender: owner-freebsd-hackers@FreeBSD.ORG Precedence: bulk List-ID: List-Archive: (Web Archive) List-Help: (List Instructions) List-Subscribe: List-Unsubscribe: X-Loop: FreeBSD.ORG On Fri, Jul 26, 2002 at 04:50:21PM +0100, Tony Finch wrote: [snip] > Comments, opinions? I'd like to commit patch two if it works satisfactorily, > otherwise I'll commit patch one. > > Patch one: > > --- main.c Fri Jul 26 15:19:06 2002 > +++ main.c.one Fri Jul 26 14:44:48 2002 > @@ -407,6 +407,8 @@ > > /* > * Modify a pointer to a filename for inplace editing and reopen stdout > + * > + * We remove the files before opening them in case of permissions problems > */ > static int > inplace_edit(filename) > @@ -453,8 +457,15 @@ > err(1, "munmap(%s)", *filename); > close(input); > close(output); > - freopen(*filename, "w", stdout); > + if (unlink(*filename) == -1) > + err(1, "unlink(%s)", *filename); > + if (freopen(*filename, "w", stdout) == NULL) fp = freopen(... > + err(1, "freopen(%s)", *filename); > + if (chmod(*filename, orig.st_mode) == -1) fchmod(fileno(fp), ... > + err(1, "chmod(%s)", *filename); > *filename = strdup(backup); > + if (*filename == NULL) > + err(1, "malloc"); > return 0; > } > > > Patch two: > > --- main.c Fri Jul 26 15:42:32 2002 > +++ main.c.two Fri Jul 26 15:43:09 2002 > @@ -413,9 +413,7 @@ > char **filename; > { > struct stat orig; > - int input, output; > char backup[MAXPATHLEN]; > - char *buffer; > > if (lstat(*filename, &orig) == -1) > err(1, "lstat"); > @@ -425,35 +423,33 @@ > } > > if (*inplace == '\0') { > - char template[] = "/tmp/sed.XXXXXXXXXX"; > - > - output = mkstemp(template); > - if (output == -1) > - err(1, "mkstemp"); > - strlcpy(backup, template, MAXPATHLEN); > + /* > + * This is a bit of a hack: we use mkstemp() to avoid the > + * mktemp() link-time warning, although mktemp() would fit in > + * this context much better. We're only interested in getting > + * a name for use in the rename(); there aren't any security > + * issues here that don't already exist in relation to the > + * original file and its directory. > + */ > + int fd; > + strlcpy(backup, *filename, MAXPATHLEN); > + strlcat(backup, ".XXXXXXXXXX", MAXPATHLEN); > + fd = mkstemp(backup); > + if (fd == -1) > + errx(1, "could not create backup of %s", *filename); > + else > + close(fd); IMHO, both the old and the newer code are wrong. they should use getenv("TMPDIR") if any and certainly not the current directory. there is probably more room in ${TMPDIR:-/var/tmp} then in the current directory. imagine that the current fs is nearly full... for the rest of the code, same assertion than above. Cyrille. -- Cyrille Lefevre mailto:cyrille.lefevre@laposte.net To Unsubscribe: send mail to majordomo@FreeBSD.org with "unsubscribe freebsd-hackers" in the body of the message