[PATCH] s3:vfs_fileid: add "fstype/mntdir deny/allow list" option
Jeremy Allison
jra at samba.org
Fri Jan 15 00:14:45 UTC 2016
On Thu, Jan 14, 2016 at 04:31:36PM +0100, Ralph Wuerthner wrote:
> Hello,
>
> please see attached patch to fix an issue we encountered recently at a
> customer.
>
> Regards
>
> Ralph
>
> PS: an alternative solution would be to pass the file path and the
> SMB_STRUCT_STAT to the file_id_create function. This way no additional
> options would be required, but I don't know if it's worth the effort to
> change all file_id_create calls.
Ugh. I gotta say I hate this :-). So this is a fix for Samba
because NFS is utterly unreliable and causes clients to hang
in the D wait-state ?
This might be better as a local or IBM-specific patch :-).
Or is the utility worth working around in Samba ?
What do others think ?
> From 6f73035370b511a25d83f9477e9693c22ebc3aa7 Mon Sep 17 00:00:00 2001
> From: Ralph Wuerthner <ralph.wuerthner at de.ibm.com>
> Date: Tue, 12 Jan 2016 16:00:24 +0100
> Subject: [PATCH] s3:vfs_fileid: add "fstype/mntdir deny/allow list" option
>
> When using the fsname or fsid algorithm a stat() and statfs() call is
> required for all mounted file systems to generate the file_id. If e.g.
> an NFS file system is unresponsive such a call might block and the smbd
> process will become unresponsive. Add "fileid:fstype deny list",
> "fileid:fstype allow list", "fileid:mntdir deny list", and
> "fileid:mntdir allow list" options to ignore potentially unresponsive
> file systems.
>
> Signed-off-by: Ralph Wuerthner <ralph.wuerthner at de.ibm.com>
> ---
> docs-xml/manpages/vfs_fileid.8.xml | 48 +++++++++++++++++++++++++
> source3/modules/vfs_fileid.c | 68 ++++++++++++++++++++++++++++++++++++
> 2 files changed, 116 insertions(+)
>
> diff --git a/docs-xml/manpages/vfs_fileid.8.xml b/docs-xml/manpages/vfs_fileid.8.xml
> index 0482203..f4b5eba 100644
> --- a/docs-xml/manpages/vfs_fileid.8.xml
> +++ b/docs-xml/manpages/vfs_fileid.8.xml
> @@ -40,6 +40,16 @@
> generates the device number based on the configured algorithm
> (see the "fileid:algorithm" option).
> </para>
> +
> + <para>When using the fsname or fsid algorithm a
> + <command>stat()</command> and <command>statfs()</command> call is
> + required for all mounted file systems to generate the file_id. If e.g.
> + an NFS file system is unresponsive such a call might block and the smbd
> + process will become unresponsive. Use the "fileid:fstype deny list",
> + "fileid:fstype allow list", "fileid:mntdir deny list", or
> + "fileid:mntdir allow list" options to ignore potentially unresponsive
> + file systems.
> + </para>
> </refsect1>
>
>
> @@ -75,6 +85,44 @@
> </listitem>
> </varlistentry>
>
> + <varlistentry>
> + <term>fileid:fstype deny list = LIST</term>
> + <listitem>
> + <para>List of file system types to be ignored for file_id
> + generation.
> + </para>
> + </listitem>
> + </varlistentry>
> +
> + <varlistentry>
> + <term>fileid:fstype allow list = LIST</term>
> + <listitem>
> + <para>List of file system types to be allowed for file_id
> + generation. If this option is set, file system types not listed
> + here are ignored.
> + </para>
> + </listitem>
> + </varlistentry>
> +
> + <varlistentry>
> + <term>fileid:mntdir deny list = LIST</term>
> + <listitem>
> + <para>List of file system mount points to be ignored for
> + file_id generation.
> + </para>
> + </listitem>
> + </varlistentry>
> +
> + <varlistentry>
> + <term>fileid:mntdir allow list = LIST</term>
> + <listitem>
> + <para>List of file system mount points to be allowed for file_id
> + generation. If this option is set, file system mount points
> + not listed here are ignored.
> + </para>
> + </listitem>
> + </varlistentry>
> +
> </variablelist>
> </refsect1>
>
> diff --git a/source3/modules/vfs_fileid.c b/source3/modules/vfs_fileid.c
> index 25048e7..7077445 100644
> --- a/source3/modules/vfs_fileid.c
> +++ b/source3/modules/vfs_fileid.c
> @@ -38,10 +38,61 @@ struct fileid_mount_entry {
> struct fileid_handle_data {
> uint64_t (*device_mapping_fn)(struct fileid_handle_data *data,
> SMB_DEV_T dev);
> + const char **fstype_deny_list;
> + const char **fstype_allow_list;
> + const char **mntdir_deny_list;
> + const char **mntdir_allow_list;
> unsigned num_mount_entries;
> struct fileid_mount_entry *mount_entries;
> };
>
> +/* check if a mount entry is allowed based on fstype and mount directory */
> +static bool fileid_mount_entry_allowed(struct fileid_handle_data *data,
> + struct mntent *m)
> +{
> + int i;
> + const char **fstype_deny = data->fstype_deny_list;
> + const char **fstype_allow = data->fstype_allow_list;
> + const char **mntdir_deny = data->mntdir_deny_list;
> + const char **mntdir_allow = data->mntdir_allow_list;
> +
> + if (fstype_deny) {
> + for (i = 0; fstype_deny[i]; i++) {
> + if (strcmp(m->mnt_type, fstype_deny[i]) == 0) {
> + return false;
> + }
> + }
> + }
> + if (fstype_allow) {
> + for (i = 0; fstype_allow[i]; i++) {
> + if (strcmp(m->mnt_type, fstype_allow[i]) == 0) {
> + break;
> + }
> + }
> + if (fstype_allow[i] == NULL) {
> + return false;
> + }
> + }
> + if (mntdir_deny) {
> + for (i=0; mntdir_deny[i]; i++) {
> + if (strcmp(m->mnt_dir, mntdir_deny[i]) == 0) {
> + return false;
> + }
> + }
> + }
> + if (mntdir_allow) {
> + for (i=0; mntdir_allow[i]; i++) {
> + if (strcmp(m->mnt_dir, mntdir_allow[i]) == 0) {
> + break;
> + }
> + }
> + if (mntdir_allow[i] == NULL) {
> + return false;
> + }
> + }
> + return true;
> +}
> +
> /* load all the mount entries from the mtab */
> static void fileid_load_mount_entries(struct fileid_handle_data *data)
> {
> @@ -59,6 +110,10 @@ static void fileid_load_mount_entries(struct fileid_handle_data *data)
> struct statfs sfs;
> struct fileid_mount_entry *cur;
>
> + if (fileid_mount_entry_allowed(data, m) != true) {
> + DEBUG(11, ("skipping mount entry %s\n", m->mnt_dir));
> + continue;
> + }
> if (stat(m->mnt_dir, &st) != 0) continue;
> if (statfs(m->mnt_dir, &sfs) != 0) continue;
>
> @@ -216,6 +271,19 @@ static int fileid_connect(struct vfs_handle_struct *handle,
> return -1;
> }
>
> + data->fstype_deny_list =
> + lp_parm_string_list(SNUM(handle->conn), "fileid",
> + "fstype deny list", NULL);
> + data->fstype_allow_list =
> + lp_parm_string_list(SNUM(handle->conn), "fileid",
> + "fstype allow list", NULL);
> + data->mntdir_deny_list =
> + lp_parm_string_list(SNUM(handle->conn), "fileid",
> + "mntdir deny list", NULL);
> + data->mntdir_allow_list =
> + lp_parm_string_list(SNUM(handle->conn), "fileid",
> + "mntdir allow list", NULL);
> +
> SMB_VFS_HANDLE_SET_DATA(handle, data, NULL,
> struct fileid_handle_data,
> return -1);
> --
> 1.7.9.5
>
More information about the samba-technical
mailing list