All the mail mirrored from lore.kernel.org
 help / color / mirror / Atom feed
From: Rasmus Villemoes <rasmus.villemoes@prevas.dk>
To: util-linux@vger.kernel.org
Cc: Masatake YAMATO <yamato@redhat.com>, Karel Zak <kzak@redhat.com>,
	Rasmus Villemoes <rasmus.villemoes@prevas.dk>
Subject: [PATCH v3] flock: add support for using fcntl() with open file description locks
Date: Thu, 25 Apr 2024 11:44:17 +0200	[thread overview]
Message-ID: <20240425094417.1174162-1-rasmus.villemoes@prevas.dk> (raw)

Currently, there is no way for shell scripts to safely access
resources protected by POSIX locking (fcntl with the F_SETLK/F_SETLKW
commands). For example, the glibc function lckpwdf(), used to
protect access to the /etc/shadow database, works by taking a
F_SETLKW on /etc/.pwd.lock .

Due to the odd semantics of POSIX locking (e.g. released when any file
descriptor associated to the inode is closed), we cannot usefully
directly expose the POSIX F_SETLK/F_SETLKW commands. However, linux
3.15 introduced F_OFD_SETLK[W], with semantics wrt. ownership and
release better matching those of flock(2), and crucially they do
conflict with locks obtained via F_SETLK[W]. With this, a shell script
can do

  exec 4> /etc/.pwd.lock
  flock --fcntl 4
  <access/modify /etc/shadow ...>
  flock --fcntl --unlock 4 # or just exit

without conflicting with passwd(1) or other utilities that
access/modify /etc/shadow.

No single-letter shorthand is defined for the option, because this is
somewhat low-level and the user really needs to know what he is doing.

Also, this leaves the door open for teaching --fcntl to accept an
optional argument: "ofd", the default, and "posix", should anyone find
a use for flock(1) taking a F_SETLK[W] lock.

Signed-off-by: Rasmus Villemoes <rasmus.villemoes@prevas.dk>
---
v3:

- Replace configure-time checking for F_OFD_ by just hard-coding the
  proper values in flock.c if the system headers don't provide them.

- Consequently, drop all HAVE_FCNTL_OFD_LOCKS guards.

v2:

- Shorten option name to --fcntl instead of --fcntl-ofd.

- Use a do_lock() helper function switching on the API to use, making
  the while () condition easier to read and making it simpler to add
  the mentioned --fcntl=posix should the need arise.

- Fix up places that need HAVE_FCNTL_OFD_LOCKS guarding.

 sys-utils/flock.c | 78 +++++++++++++++++++++++++++++++++++++++++++++--
 1 file changed, 76 insertions(+), 2 deletions(-)

diff --git a/sys-utils/flock.c b/sys-utils/flock.c
index 7d878ff81..c42c9da51 100644
--- a/sys-utils/flock.c
+++ b/sys-utils/flock.c
@@ -48,6 +48,17 @@
 #include "monotonic.h"
 #include "timer.h"
 
+#ifndef F_OFD_GETLK
+#define F_OFD_GETLK	36
+#define F_OFD_SETLK	37
+#define F_OFD_SETLKW	38
+#endif
+
+enum {
+	API_FLOCK,
+	API_FCNTL_OFD,
+};
+
 static void __attribute__((__noreturn__)) usage(void)
 {
 	fputs(USAGE_HEADER, stdout);
@@ -70,6 +81,7 @@ static void __attribute__((__noreturn__)) usage(void)
 	fputs(_(  " -o, --close              close file descriptor before running command\n"), stdout);
 	fputs(_(  " -c, --command <command>  run a single command string through the shell\n"), stdout);
 	fputs(_(  " -F, --no-fork            execute command without forking\n"), stdout);
+	fputs(_(  "     --fcntl              use fcntl(F_OFD_SETLK) rather than flock()\n"), stdout);
 	fputs(_(  "     --verbose            increase verbosity\n"), stdout);
 	fputs(USAGE_SEPARATOR, stdout);
 	fprintf(stdout, USAGE_HELP_OPTIONS(26));
@@ -126,6 +138,49 @@ static void __attribute__((__noreturn__)) run_program(char **cmd_argv)
 	_exit((errno == ENOMEM) ? EX_OSERR : EX_UNAVAILABLE);
 }
 
+static int flock_to_fcntl_type(int op)
+{
+	switch (op) {
+	case LOCK_EX:
+		return F_WRLCK;
+	case LOCK_SH:
+		return F_RDLCK;
+	case LOCK_UN:
+		return F_UNLCK;
+	default:
+		errx(EX_SOFTWARE, _("internal error, unknown operation %d"), op);
+	}
+}
+
+static int fcntl_lock(int fd, int op, int block)
+{
+	struct flock arg = {
+		.l_type = flock_to_fcntl_type(op),
+		.l_whence = SEEK_SET,
+		.l_start = 0,
+		.l_len = 0,
+	};
+	int cmd = (block & LOCK_NB) ? F_OFD_SETLK : F_OFD_SETLKW;
+	return fcntl(fd, cmd, &arg);
+}
+
+static int do_lock(int api, int fd, int op, int block)
+{
+	switch (api) {
+	case API_FLOCK:
+		return flock(fd, op | block);
+	case API_FCNTL_OFD:
+		return fcntl_lock(fd, op, block);
+	/*
+	 * Should never happen, api can never have values other than
+	 * API_*.
+	 */
+	default:
+		errx(EX_SOFTWARE, _("internal error, unknown api %d"), api);
+	}
+}
+
+
 int main(int argc, char *argv[])
 {
 	struct ul_timer timer;
@@ -140,6 +195,7 @@ int main(int argc, char *argv[])
 	int no_fork = 0;
 	int status;
 	int verbose = 0;
+	int api = API_FLOCK;
 	struct timeval time_start = { 0 }, time_done = { 0 };
 	/*
 	 * The default exit code for lock conflict or timeout
@@ -149,7 +205,8 @@ int main(int argc, char *argv[])
 	char **cmd_argv = NULL, *sh_c_argv[4];
 	const char *filename = NULL;
 	enum {
-		OPT_VERBOSE = CHAR_MAX + 1
+		OPT_VERBOSE = CHAR_MAX + 1,
+		OPT_FCNTL,
 	};
 	static const struct option long_options[] = {
 		{"shared", no_argument, NULL, 's'},
@@ -163,6 +220,7 @@ int main(int argc, char *argv[])
 		{"close", no_argument, NULL, 'o'},
 		{"no-fork", no_argument, NULL, 'F'},
 		{"verbose", no_argument, NULL, OPT_VERBOSE},
+		{"fcntl", no_argument, NULL, OPT_FCNTL},
 		{"help", no_argument, NULL, 'h'},
 		{"version", no_argument, NULL, 'V'},
 		{NULL, 0, NULL, 0}
@@ -217,6 +275,9 @@ int main(int argc, char *argv[])
 			if (conflict_exit_code < 0 || conflict_exit_code > 255)
 				errx(EX_USAGE, _("exit code out of range (expected 0 to 255)"));
 			break;
+		case OPT_FCNTL:
+			api = API_FCNTL_OFD;
+			break;
 		case OPT_VERBOSE:
 			verbose = 1;
 			break;
@@ -234,6 +295,13 @@ int main(int argc, char *argv[])
 		errx(EX_USAGE,
 			_("the --no-fork and --close options are incompatible"));
 
+	/*
+	 * For fcntl(F_OFD_SETLK), an exclusive lock requires that the
+	 * file is open for write.
+	 */
+	if (api != API_FLOCK && type == LOCK_EX)
+		open_flags = O_WRONLY;
+
 	if (argc > optind + 1) {
 		/* Run command */
 		if (!strcmp(argv[optind + 1], "-c") ||
@@ -280,9 +348,15 @@ int main(int argc, char *argv[])
 
 	if (verbose)
 		gettime_monotonic(&time_start);
-	while (flock(fd, type | block)) {
+	while (do_lock(api, fd, type, block)) {
 		switch (errno) {
 		case EWOULDBLOCK:
+			/*
+			 * Per the man page, for fcntl(), EACCES may
+			 * be returned and means the same as
+			 * EAGAIN/EWOULDBLOCK.
+			 */
+		case EACCES:
 			/* -n option set and failed to lock. */
 			if (verbose)
 				warnx(_("failed to get lock"));
-- 
2.40.1.1.g1c60b9335d


             reply	other threads:[~2024-04-25  9:44 UTC|newest]

Thread overview: 4+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2024-04-25  9:44 Rasmus Villemoes [this message]
2024-04-26  8:01 ` [PATCH v3] flock: add support for using fcntl() with open file description locks Karel Zak
2024-04-28 21:07 ` Thomas Weißschuh
2024-04-29  8:29   ` Karel Zak

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=20240425094417.1174162-1-rasmus.villemoes@prevas.dk \
    --to=rasmus.villemoes@prevas.dk \
    --cc=kzak@redhat.com \
    --cc=util-linux@vger.kernel.org \
    --cc=yamato@redhat.com \
    /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.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.