Samba and Linux OFD locks
Andreas Schneider
asn at samba.org
Thu Jan 31 14:35:01 UTC 2019
On Wednesday, January 30, 2019 7:45:30 PM CET Andreas Schneider via samba-
technical wrote:
> Hello,
>
> on newer Linux distributions with glibc >= 2.20 we have OFD file locks
> available. Our autobuild hosts are too old!
>
> However if your system supports OFD file locks the
> samba3.vfs.fruit_netatalk.nt4 test is failing!
attached is a patchset which does some cleanup and disables OFD locks. They
are obviously not working. The code might need some fixes.
I've talked to Jeff Layton who implemented OFD locks. The follwing strace
line:
3163 19:20:18.349723 fcntl(37</home/asn/workspace/projects/samba/st/nt4_dc/
share/vfs_fruit_dir/locking_conflict.txt>, F_OFD_GETLK, {l_type=F_RDLCK,
l_whence=SEEK_SET, l_start=0, l_len=9223372036854775807,
l_pid=18446744073709551615}) = 0 <0.000003>
tells us that there is still a read lock on the file! The l_pid has this bogus
value because in principle if you had a file description shared across
processes, you can't really say which pid owns it.
Looking at the strace we seem to open and lock the file twice!
3163 19:20:18.348328 fcntl(36</home/asn/workspace/projects/samba/st/nt4_dc/
share/vfs_fruit_dir/locking_conflict.txt>, F_OFD_SETLK, {l_type=F_RDLCK,
l_whence=SEEK_SET, l_start=0, l_len=9223372036854775807}) = 0 <0.000004>
This line locks it and there is no release! Also we create a lock on file
descriptor 36 and 37. OFD locks are different from POSIX locks in that if you
set a lock on two different fds for the same file they'll conflict with one
another!
Here is the merge request with a pipeline running:
https://gitlab.com/samba-team/samba/merge_requests/234
Cheers,
Andreas
--
Andreas Schneider asn at samba.org
Samba Team www.samba.org
GPG-ID: 8DFF53E18F2ABC8D8F3C92237EE0FC4DCC014E3D
-------------- next part --------------
>From d964ce0c6fc038397155b9596103b42c365b968c Mon Sep 17 00:00:00 2001
From: Andreas Schneider <asn at samba.org>
Date: Wed, 30 Jan 2019 18:09:52 +0100
Subject: [PATCH 1/3] s3:vfs: Initialize pid to 0 in test_netatalk_lock()
Signed-off-by: Andreas Schneider <asn at samba.org>
---
source3/modules/vfs_fruit.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/source3/modules/vfs_fruit.c b/source3/modules/vfs_fruit.c
index 9f3fe24e5fc..0a67ad39d8e 100644
--- a/source3/modules/vfs_fruit.c
+++ b/source3/modules/vfs_fruit.c
@@ -2647,7 +2647,7 @@ static bool test_netatalk_lock(files_struct *fsp, off_t in_offset)
off_t offset = in_offset;
off_t len = 1;
int type = F_WRLCK;
- pid_t pid;
+ pid_t pid = 0;
result = SMB_VFS_GETLOCK(fsp, &offset, &len, &type, &pid);
if (result == false) {
--
2.20.1
>From 02d4c954923b0be3950660fa695cce53919c1f24 Mon Sep 17 00:00:00 2001
From: Andreas Schneider <asn at samba.org>
Date: Wed, 30 Jan 2019 18:45:34 +0100
Subject: [PATCH 2/3] s3:vfs: Correctly check if OFD locks should be enabled or
not
Also the smb.conf options should only be checked once and a reload of
the config should not switch to a different locking mode.
Signed-off-by: Andreas Schneider <asn at samba.org>
---
source3/include/proto.h | 2 +-
source3/lib/util.c | 7 ++-----
source3/modules/vfs_default.c | 14 ++++----------
source3/smbd/files.c | 9 +++++++++
4 files changed, 16 insertions(+), 16 deletions(-)
diff --git a/source3/include/proto.h b/source3/include/proto.h
index 715bc56e286..9d6192967ba 100644
--- a/source3/include/proto.h
+++ b/source3/include/proto.h
@@ -360,7 +360,7 @@ void set_namearray(name_compare_entry **ppname_array, const char *namelist);
void free_namearray(name_compare_entry *name_array);
bool fcntl_lock(int fd, int op, off_t offset, off_t count, int type);
bool fcntl_getlock(int fd, int op, off_t *poffset, off_t *pcount, int *ptype, pid_t *ppid);
-int map_process_lock_to_ofd_lock(int op, bool *use_ofd_locks);
+int map_process_lock_to_ofd_lock(int op);
bool is_myname(const char *s);
void ra_lanman_string( const char *native_lanman );
const char *get_remote_arch_str(void);
diff --git a/source3/lib/util.c b/source3/lib/util.c
index 5dbd67349fa..7530ea67973 100644
--- a/source3/lib/util.c
+++ b/source3/lib/util.c
@@ -1079,7 +1079,7 @@ bool fcntl_getlock(int fd, int op, off_t *poffset, off_t *pcount, int *ptype, pi
}
#if defined(HAVE_OFD_LOCKS)
-int map_process_lock_to_ofd_lock(int op, bool *use_ofd_locks)
+int map_process_lock_to_ofd_lock(int op)
{
switch (op) {
case F_GETLK:
@@ -1095,16 +1095,13 @@ int map_process_lock_to_ofd_lock(int op, bool *use_ofd_locks)
op = F_OFD_SETLKW;
break;
default:
- *use_ofd_locks = false;
return -1;
}
- *use_ofd_locks = true;
return op;
}
#else /* HAVE_OFD_LOCKS */
-int map_process_lock_to_ofd_lock(int op, bool *use_ofd_locks)
+int map_process_lock_to_ofd_lock(int op)
{
- *use_ofd_locks = false;
return op;
}
#endif /* HAVE_OFD_LOCKS */
diff --git a/source3/modules/vfs_default.c b/source3/modules/vfs_default.c
index a27d33a6bea..cb5537e096e 100644
--- a/source3/modules/vfs_default.c
+++ b/source3/modules/vfs_default.c
@@ -2553,11 +2553,8 @@ static bool vfswrap_lock(vfs_handle_struct *handle, files_struct *fsp, int op, o
START_PROFILE(syscall_fcntl_lock);
- if (fsp->use_ofd_locks || !lp_parm_bool(SNUM(fsp->conn),
- "smbd",
- "force process locks",
- false)) {
- op = map_process_lock_to_ofd_lock(op, &fsp->use_ofd_locks);
+ if (fsp->use_ofd_locks) {
+ op = map_process_lock_to_ofd_lock(op);
}
result = fcntl_lock(fsp->fh->fd, op, offset, count, type);
@@ -2581,11 +2578,8 @@ static bool vfswrap_getlock(vfs_handle_struct *handle, files_struct *fsp, off_t
START_PROFILE(syscall_fcntl_getlock);
- if (fsp->use_ofd_locks || !lp_parm_bool(SNUM(fsp->conn),
- "smbd",
- "force process locks",
- false)) {
- op = map_process_lock_to_ofd_lock(op, &fsp->use_ofd_locks);
+ if (fsp->use_ofd_locks) {
+ op = map_process_lock_to_ofd_lock(op);
}
result = fcntl_getlock(fsp->fh->fd, op, poffset, pcount, ptype, ppid);
diff --git a/source3/smbd/files.c b/source3/smbd/files.c
index 397baea84cb..99b4937c99b 100644
--- a/source3/smbd/files.c
+++ b/source3/smbd/files.c
@@ -51,6 +51,15 @@ NTSTATUS fsp_new(struct connection_struct *conn, TALLOC_CTX *mem_ctx,
goto fail;
}
+#if defined(HAVE_OFD_LOCKS)
+ fsp->use_ofd_locks = true;
+ if (lp_parm_bool(SNUM(conn),
+ "smbd",
+ "force process locks",
+ false)) {
+ fsp->use_ofd_locks = false;
+ }
+#endif
fsp->fh->ref_count = 1;
fsp->fh->fd = -1;
--
2.20.1
>From f3b1e4176bc4e193ac29742cb0a83a400495bde9 Mon Sep 17 00:00:00 2001
From: Andreas Schneider <asn at samba.org>
Date: Thu, 31 Jan 2019 14:58:36 +0100
Subject: [PATCH 3/3] s3:smbd: Disable OFD locks by default
Those don't seem to be working and currently are just mapped and we
assume they they have the same semantics as POSIX locks. However the
manual states they don't.
Signed-off-by: Andreas Schneider <asn at samba.org>
---
source3/smbd/files.c | 7 ++++++-
1 file changed, 6 insertions(+), 1 deletion(-)
diff --git a/source3/smbd/files.c b/source3/smbd/files.c
index 99b4937c99b..099277745b8 100644
--- a/source3/smbd/files.c
+++ b/source3/smbd/files.c
@@ -52,7 +52,12 @@ NTSTATUS fsp_new(struct connection_struct *conn, TALLOC_CTX *mem_ctx,
}
#if defined(HAVE_OFD_LOCKS)
- fsp->use_ofd_locks = true;
+ /*
+ * FIXME: OFD locks are disabled by default till someone implements
+ * and tests them correctly.
+ */
+ fsp->use_ofd_locks = false;
+
if (lp_parm_bool(SNUM(conn),
"smbd",
"force process locks",
--
2.20.1
More information about the samba-technical
mailing list