Source-Changes-HG archive
[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index][Old Index]
[src/trunk]: src/usr.bin/shlock PR/56112: Justin Parrott: Don't unlink a lock...
details: https://anonhg.NetBSD.org/src/rev/2b5b32c92d65
branches: trunk
changeset: 378551:2b5b32c92d65
user: christos <christos%NetBSD.org@localhost>
date: Fri Apr 16 22:41:12 2021 +0000
description:
PR/56112: Justin Parrott: Don't unlink a lock file without locking it, because
you can have races when multiple processes try to unlink it. Check the link
count to see if we have won the race.
While here:
- use snprintf
- use warn
- use size_t/ssize_t
- use loops instead of goto
- KNF
diffstat:
usr.bin/shlock/shlock.c | 211 ++++++++++++++++++++++++++++++-----------------
1 files changed, 134 insertions(+), 77 deletions(-)
diffs (truncated from 351 to 300 lines):
diff -r cfa7f2fd9074 -r 2b5b32c92d65 usr.bin/shlock/shlock.c
--- a/usr.bin/shlock/shlock.c Fri Apr 16 18:31:28 2021 +0000
+++ b/usr.bin/shlock/shlock.c Fri Apr 16 22:41:12 2021 +0000
@@ -1,4 +1,4 @@
-/* $NetBSD: shlock.c,v 1.13 2015/04/10 09:34:43 tron Exp $ */
+/* $NetBSD: shlock.c,v 1.14 2021/04/16 22:41:12 christos Exp $ */
/*-
* Copyright (c) 2006 The NetBSD Foundation, Inc.
@@ -64,11 +64,13 @@
#include <sys/cdefs.h>
#ifndef lint
-__RCSID("$NetBSD: shlock.c,v 1.13 2015/04/10 09:34:43 tron Exp $");
+__RCSID("$NetBSD: shlock.c,v 1.14 2021/04/16 22:41:12 christos Exp $");
#endif
#include <sys/types.h>
+#include <sys/stat.h>
#include <sys/file.h>
+#include <err.h>
#include <fcntl.h> /* Needed on hpux */
#include <stdio.h>
#include <signal.h>
@@ -91,8 +93,10 @@
static int Debug = FALSE;
static char *Pname;
static const char USAGE[] = "%s: USAGE: %s [-du] [-p PID] -f file\n";
-static const char E_unlk[] = "%s: unlink(%s): %s\n";
-static const char E_open[] = "%s: open(%s): %s\n";
+static const char E_unlk[] = "unlink(%s)";
+static const char E_link[] = "link(%s, %s)";
+static const char E_open[] = "open(%s)";
+static const char E_stat[] = "stat(%s)";
#define dprintf if (Debug) printf
@@ -103,10 +107,10 @@ static const char E_open[] = "%s: open(%
*/
/* the following is in case you need to make the prototypes go away. */
-static char *xtmpfile(char *, pid_t, int);
+static char *xtmpfile(const char *, pid_t, int);
static int p_exists(pid_t);
-static int cklock(char *, int);
-static int mklock(char *, pid_t, int);
+static int cklock(const char *, struct stat *, int);
+static int mklock(const char *, pid_t, int);
__dead static void bad_usage(void);
/*
@@ -116,14 +120,14 @@ static int mklock(char *, pid_t, int);
** which might not be in the same filesystem.
*/
static char *
-xtmpfile(char *file, pid_t pid, int uucpstyle)
+xtmpfile(const char *file, pid_t pid, int uucpstyle)
{
int fd;
- int len;
+ size_t len;
char *cp, buf[BUFSIZ];
static char tempname[BUFSIZ];
- sprintf(buf, "shlock%ld", (long)getpid());
+ snprintf(buf, sizeof(buf), "shlock%jd", (intmax_t)getpid());
if ((cp = strrchr(strcpy(tempname, file), '/')) != NULL) {
*++cp = '\0';
(void) strcat(tempname, buf);
@@ -131,27 +135,21 @@ xtmpfile(char *file, pid_t pid, int uucp
(void) strcpy(tempname, buf);
dprintf("%s: temporary filename: %s\n", Pname, tempname);
- sprintf(buf, "%ld\n", (long)pid);
+ snprintf(buf, sizeof(buf), "%jd\n", (intmax_t)pid);
len = strlen(buf);
-openloop:
- if ((fd = open(tempname, O_RDWR|O_CREAT|O_EXCL, 0644)) < 0) {
+ while ((fd = open(tempname, O_RDWR|O_CREAT|O_TRUNC|O_SYNC|O_EXCL, 0644))
+ == -1)
+ {
switch(errno) {
case EEXIST:
- dprintf("%s: file %s exists already.\n",
- Pname, tempname);
- if (unlink(tempname) < 0) {
- fprintf(stderr, E_unlk,
- Pname, tempname, strerror(errno));
- return (NULL);
+ if (unlink(tempname) == -1) {
+ warn(E_unlk, tempname);
+ return NULL;
}
- /*
- ** Further profanity
- */
- goto openloop;
+ break;
default:
- fprintf(stderr, E_open,
- Pname, tempname, strerror(errno));
- return (NULL);
+ warn(E_open, tempname);
+ return NULL;
}
}
@@ -162,19 +160,17 @@ openloop:
*/
if (uucpstyle ?
(write(fd, &pid, sizeof(pid)) != sizeof(pid)) :
- (write(fd, buf, len) < 0))
+ (write(fd, buf, len) != (ssize_t)len))
{
- fprintf(stderr, "%s: write(%s,%ld): %s\n",
- Pname, tempname, (long)pid, strerror(errno));
+ warn("write(%s,%jd)", tempname, (intmax_t)pid);
(void) close(fd);
- if (unlink(tempname) < 0) {
- fprintf(stderr, E_unlk,
- Pname, tempname, strerror(errno));
+ if (unlink(tempname) == -1) {
+ warn(E_unlk, tempname);
}
- return (NULL);
+ return NULL;
}
(void) close(fd);
- return(tempname);
+ return tempname;
}
/*
@@ -184,26 +180,26 @@ openloop:
static int
p_exists(pid_t pid)
{
- dprintf("%s: process %ld is ", Pname, (long)pid);
+ dprintf("%s: process %jd is ", Pname, (intmax_t)pid);
if (pid <= 0) {
dprintf("invalid\n");
- return(FALSE);
+ return FALSE;
}
- if (kill(pid, 0) < 0) {
+ if (kill(pid, 0) == -1) {
switch(errno) {
case ESRCH:
- dprintf("dead\n");
- return(FALSE); /* pid does not exist */
+ dprintf("dead %jd\n", (intmax_t)pid);
+ return FALSE; /* pid does not exist */
case EPERM:
dprintf("alive\n");
- return(TRUE); /* pid exists */
+ return TRUE; /* pid exists */
default:
dprintf("state unknown: %s\n", strerror(errno));
- return(TRUE); /* be conservative */
+ return TRUE; /* be conservative */
}
}
dprintf("alive\n");
- return(TRUE); /* pid exists */
+ return TRUE; /* pid exists */
}
/*
@@ -222,7 +218,7 @@ p_exists(pid_t pid)
**
*/
static int
-cklock(char *file, int uucpstyle)
+cklock(const char *file, struct stat *st, int uucpstyle)
{
int fd = open(file, O_RDONLY);
ssize_t len;
@@ -230,10 +226,16 @@ cklock(char *file, int uucpstyle)
char buf[BUFSIZ];
dprintf("%s: checking extant lock <%s>\n", Pname, file);
- if (fd < 0) {
+ if (fd == -1) {
if (errno != ENOENT)
- fprintf(stderr, E_open, Pname, file, strerror(errno));
- return(TRUE); /* might or might not; conservatism */
+ warn(E_open, file);
+ return TRUE; /* might or might not; conservatism */
+ }
+
+ if (st != NULL && fstat(fd, st) == -1) {
+ warn(E_stat, file);
+ close(fd);
+ return TRUE;
}
if (uucpstyle ?
@@ -242,59 +244,114 @@ cklock(char *file, int uucpstyle)
{
close(fd);
dprintf("%s: lock file format error\n", Pname);
- return(FALSE);
+ return FALSE;
}
close(fd);
buf[len + 1] = '\0';
- return(p_exists(uucpstyle ? pid : atoi(buf)));
+ return p_exists(uucpstyle ? pid : atoi(buf));
}
static int
-mklock(char *file, pid_t pid, int uucpstyle)
+mklock(const char *file, pid_t pid, int uucpstyle)
{
- char *tmp;
+ char *tmp, tmp2[BUFSIZ];
int retcode = FALSE;
+ struct stat stlock, sttmp, stlock2;
- dprintf("%s: trying lock <%s> for process %ld\n", Pname, file,
- (long)pid);
+ dprintf("%s: trying lock <%s> for process %jd\n", Pname, file,
+ (intmax_t)pid);
if ((tmp = xtmpfile(file, pid, uucpstyle)) == NULL)
- return(FALSE);
+ return FALSE;
-linkloop:
- if (link(tmp, file) < 0) {
+ while (link(tmp, file) == -1) {
switch(errno) {
case EEXIST:
dprintf("%s: lock <%s> already exists\n", Pname, file);
- if (cklock(file, uucpstyle)) {
+ if (cklock(file, &stlock, uucpstyle)) {
dprintf("%s: extant lock is valid\n", Pname);
- break;
- } else {
- dprintf("%s: lock is invalid, removing\n",
- Pname);
- if (unlink(file) < 0) {
- fprintf(stderr, E_unlk,
- Pname, file, strerror(errno));
- break;
- }
+ goto out;
+ }
+ if (stat(tmp, &sttmp) == -1) {
+ warn(E_stat, tmp);
+ goto out;
}
/*
- ** I hereby profane the god of structured programming,
- ** Edsgar Dijkstra
- */
- goto linkloop;
+ * We need to take another lock against the lock
+ * file to protect it against multiple removals
+ */
+ snprintf(tmp2, sizeof(tmp2), "%s-2", tmp);
+ if (unlink(tmp2) == -1 && errno != ENOENT) {
+ warn(E_unlk, tmp2);
+ goto out;
+ }
+ if (stlock.st_ctime >= sttmp.st_ctime) {
+ dprintf("%s: lock time changed %jd >= %jd\n",
+ Pname, (intmax_t)stlock.st_ctime,
+ (intmax_t)stlock2.st_ctime);
+ goto out;
+ }
+ if (stlock.st_nlink != 1) {
+ dprintf("%s: someone else linked to it %d\n",
+ Pname, stlock.st_nlink);
+ goto out;
+ }
+ if (link(file, tmp2) == -1) {
+ /* someone took our temp name! */
+ warn(E_link, file, tmp2);
+ goto out2;
+ }
+ if (stat(file, &stlock2) == -1) {
+ warn(E_stat, file);
+ goto out2;
+ }
+ if (stlock.st_ino != stlock2.st_ino) {
+ dprintf("%s: lock inode changed %jd != %jd\n",
+ Pname, (intmax_t)stlock.st_ino,
+ (intmax_t)stlock2.st_ino);
+ goto out2;
+ }
+ if (stlock2.st_nlink != 2) {
+ dprintf("%s: someone else linked to it %d\n",
+ Pname, stlock2.st_nlink);
+ goto out2;
+ }
+
+ dprintf("%s: lock is invalid, removing\n", Pname);
+ if (unlink(file) == -1) {
+ warn(E_unlk, file);
+ goto out2;
Home |
Main Index |
Thread Index |
Old Index