Date: Sun, 6 Aug 2000 11:58:06 -0700 (PDT) From: jkuroda@CS.Berkeley.EDU To: FreeBSD-gnats-submit@freebsd.org Subject: bin/20445: restore(8) uses non-varying filenames for /tmp files when restoring given filesystem repeatedly Message-ID: <200008061858.LAA24180@sushi.CS.Berkeley.EDU>
next in thread | raw e-mail | index | archive | help
>Number: 20445
>Category: bin
>Synopsis: restore(8) uses non-varying filenames for /tmp files when restoring given filesystem repeatedly
>Confidential: no
>Severity: non-critical
>Priority: low
>Responsible: freebsd-bugs
>State: open
>Quarter:
>Keywords:
>Date-Required:
>Class: sw-bug
>Submitter-Id: current-users
>Arrival-Date: Sun Aug 06 12:00:04 PDT 2000
>Closed-Date:
>Last-Modified:
>Originator: Jon Masami Kuroda
>Release: FreeBSD 3.3-STABLE i386
>Organization:
University of California at Berkeley, EECS
>Environment:
"problem" discovered while cloning filesystems via restore(8) with
the -R or -r flags to multiple disks from single dumpfile created
via dump(8).
# for newroot in 1 2 3 4 5; do
cd /mnt/altroot/${newroot}/ && restore rf /scratch/root.dump &
done
>Description:
restore(8) uses the dumpdate of the filesystem being restored to
create the filenames for the files in /tmp used to store the names
of directories -- /tmp/rst* -- and of their ownership/mode/time stamps
-- /tmp/rstmode*. The first file, dirfile is used to store the list
of dirs to be created before files are restored into them, the second,
modefile, to restore the ownership,mode, and time stamps on those
dirs after file restores are finished. The dumpdate is the number
of seconds since the Unix Epoch (tm) till the time the filesystem
in question was dumped.
example:
dirs.c:
1 /*
* Extract directory contents, building up a directory structure
* on disk for extraction by name.
* If genmode is requested, save mode, owner, and times for all
5 * directories on the tape.
*/
void
extractdirs(genmode)
int genmode;
10 {
register int i;
register struct dinode *ip;
struct inotab *itp;
struct direct nulldir;
15 int fd;
vprintf(stdout, "Extract directories from tape\n");
(void) sprintf(dirfile, "%srstdir%d", _PATH_TMP, dumpdate);
20 if (command != 'r' && command != 'R') {
(void *) strcat(dirfile, "-XXXXXX");
fd = mkstemp(dirfile);
} else
fd = open(dirfile, O_RDWR|O_CREAT|O_EXCL, 0666);
25 if (fd == -1 || (df = fdopen(fd, "w")) == NULL) {
if (fd != -1)
close(fd);
warn("%s - cannot create directory temporary\nfopen", dirfile);
done(1);
30 }
if (genmode != 0) {
(void) sprintf(modefile, "%srstmode%d", _PATH_TMP, dumpdate);
if (command != 'r' && command != 'R') {
(void *) strcat(modefile, "-XXXXXX");
35 fd = mkstemp(modefile);
} else
fd = open(modefile, O_RDWR|O_CREAT|O_EXCL, 0666);
if (fd == -1 || (mf = fdopen(fd, "w")) == NULL) {
if (fd != -1)
40 close(fd);
warn("%s - cannot create modefile\nfopen", modefile);
done(1);
}
44 }
The dumpdate made of the filenames prior to their creation in lines
19 and 32. Note that after that, a call to mkstemp(3) is made but
*only* if neither the "R" or "r" flags are provided to restore(8).
Else, the file is created via open(2) with mode 0666, and umask 077
obtained from main.c:93.
This introduces one very annoying problem.
The name of restore(8)'s temporary files when restore(8) is given
the "R" or "r" flags are very predictable -- in fact, they never
change. While the use of the O_CREAT and O_EXCL flags to open(2)
should prevent a non-privileged user from exploiting an otherwise
possible race condition (right?), the problem I found is that this
prevents one from using ``restore rf'' or ``restore Rf'' to clone a
filesystem to several blank filesystems in parallel from a single
dumpfile created earlier by dump(8) as each restore will attempt
to create the same filenames in /tmp and only the first one will
succeed. (given the example shown above)
The rest will fail out like so:
restore: /tmp/rstdir965378984 - cannot create directory temporary
fopen: File exists
restore: /tmp/rstmode965378984 - cannot create modefile
fopen: File exists
and so prevent one from restoring from one dumpfile to multiple
filesystems (such as in FS cloning) simultaneously.
It seems that mkstemp(3) was added in revision 1.6.2.1, but mkstemp(3)
was used only in certain places, presumably only those places that
would introduce nasty race conditions.
>How-To-Repeat:
* given a number of newfs'd fs's mounted in /mnt/altroot
* and a dump of an example root fs to be cloned in /scratch/...
# for newroot in 1 2 3 4 5; do
cd /mnt/altroot/${newroot}/ && restore rf /scratch/root.dump &
done
watch the 2nd restore and onward complain about not being to
open temporary files correctly.
>Fix:
Use mkstemp(3) instead of open(2) by itself in extractdirs(),
when creating "dirfile" and "modefile". Do not recreate the
value of "modefile" from "dumpdate" in setdirmodes(), and instead
rely on mkstemp to modify the value of "modefile", back in extractdirs(), which is in an array of "static char"'s. This
can easily be accomplished by removing the special casing for
the "R" and "r" flags and letting these cases reap the benefits
of mkstemp(3) as in other case such as when the "x" flag is used.
I scoured the code and the CVS logs and couldn't find a reason
for why mkstemp shouldn't be used here. I think it makes sense
to on its own merit and also because it makes the code slightly
simpler for not having to special case "R" and "r". But, I
could be mistaken in this and my patch below could be terribly
naive, so take it all with a grain of salt and the 10 coke's I
had prior to writing this up.
Thanks to Mike Smith for helping me remember that I had been
meaning to submit a PR for this for sometime now.
A first pass at patch on ${SRC}/sbin/restore/dirs.c
This patch tested on 3.4-STABLE and 4.1-RELEASE.
*** dirs.c Sun Aug 6 11:46:31 2000
--- dirs.c.moremkstemp Sun Aug 6 11:46:22 2000
***************
*** 148,158 ****
vprintf(stdout, "Extract directories from tape\n");
(void) sprintf(dirfile, "%srstdir%d", _PATH_TMP, dumpdate);
! if (command != 'r' && command != 'R') {
(void *) strcat(dirfile, "-XXXXXX");
fd = mkstemp(dirfile);
! } else
! fd = open(dirfile, O_RDWR|O_CREAT|O_EXCL, 0666);
if (fd == -1 || (df = fdopen(fd, "w")) == NULL) {
if (fd != -1)
close(fd);
--- 148,158 ----
vprintf(stdout, "Extract directories from tape\n");
(void) sprintf(dirfile, "%srstdir%d", _PATH_TMP, dumpdate);
! /* if (command != 'r' && command != 'R') { */
(void *) strcat(dirfile, "-XXXXXX");
fd = mkstemp(dirfile);
! /*} else
! fd = open(dirfile, O_RDWR|O_CREAT|O_EXCL, 0666); */
if (fd == -1 || (df = fdopen(fd, "w")) == NULL) {
if (fd != -1)
close(fd);
***************
*** 161,171 ****
}
if (genmode != 0) {
(void) sprintf(modefile, "%srstmode%d", _PATH_TMP, dumpdate);
! if (command != 'r' && command != 'R') {
(void *) strcat(modefile, "-XXXXXX");
fd = mkstemp(modefile);
! } else
! fd = open(modefile, O_RDWR|O_CREAT|O_EXCL, 0666);
if (fd == -1 || (mf = fdopen(fd, "w")) == NULL) {
if (fd != -1)
close(fd);
--- 161,171 ----
}
if (genmode != 0) {
(void) sprintf(modefile, "%srstmode%d", _PATH_TMP, dumpdate);
! /* if (command != 'r' && command != 'R') { */
(void *) strcat(modefile, "-XXXXXX");
fd = mkstemp(modefile);
! /* } else
! fd = open(modefile, O_RDWR|O_CREAT|O_EXCL, 0666); */
if (fd == -1 || (mf = fdopen(fd, "w")) == NULL) {
if (fd != -1)
close(fd);
***************
*** 594,601 ****
char *cp;
vprintf(stdout, "Set directory mode, owner, and times.\n");
! if (command == 'r' || command == 'R')
! (void) sprintf(modefile, "%srstmode%d", _PATH_TMP, dumpdate);
if (modefile[0] == '#') {
panic("modefile not defined\n");
fprintf(stderr, "directory mode, owner, and times not set\n");
--- 594,601 ----
char *cp;
vprintf(stdout, "Set directory mode, owner, and times.\n");
! /* if (command == 'r' || command == 'R')
! (void) sprintf(modefile, "%srstmode%d", _PATH_TMP, dumpdate); */
if (modefile[0] == '#') {
panic("modefile not defined\n");
fprintf(stderr, "directory mode, owner, and times not set\n");
>Release-Note:
>Audit-Trail:
>Unformatted:
To Unsubscribe: send mail to majordomo@FreeBSD.org
with "unsubscribe freebsd-bugs" in the body of the message
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?200008061858.LAA24180>