[PATCH] avoid stat of foreign file systems in quota code
Uri Simchoni
uri at samba.org
Tue Jan 26 06:25:38 UTC 2016
Resubmitting patch per Jeremy's request.
On 01/26/2016 12:08 AM, Uri Simchoni wrote:
> Hi,
> Anyone cares to comment?
>
> Thanks,
> Uri.
>
> On 01/19/2016 12:07 AM, Uri Simchoni wrote:
>> Hi,
>>
>> Following the discussion on vfs_fileid, I realized that we may have
>> the same issue in the quota code. Whenever you type "dir" at a
>> Windows command prompt on a samba share, the client asks for free
>> space and activates the quota code.
>>
>> Attached is a fix that avoids stating mounted file systems that
>> cannot be "ours".
>>
>> - This patch is independent of the refactoring patch series that I
>> just re-sent
>> - I haven't actually gone through the effort of verifying that there
>> is an issue, reproducing it, and seeing that the patch fixes it. Just
>> verified no degradation on my system. So I'm not sure whether this
>> patch falls under "bugfix" (that should be backported if it's good)
>> or "eliminating not-so-nice behavior".
>>
>> Thanks,
>> Uri
>>
>
>
-------------- next part --------------
From 59e1f102720e4670018c7bff72b09095a0940121 Mon Sep 17 00:00:00 2001
From: Uri Simchoni <uri at samba.org>
Date: Mon, 18 Jan 2016 23:34:06 +0200
Subject: [PATCH 1/2] s3-lib: introduce sys_realpath()
Add sys_realpath() function that captures the OS variations
on realpath().
Signed-off-by: Uri Simchoni <uri at samba.org>
---
source3/include/proto.h | 1 +
source3/lib/system.c | 25 +++++++++++++++++++++++++
source3/modules/vfs_default.c | 15 +--------------
3 files changed, 27 insertions(+), 14 deletions(-)
diff --git a/source3/include/proto.h b/source3/include/proto.h
index 809cb95..cd54ee6 100644
--- a/source3/include/proto.h
+++ b/source3/include/proto.h
@@ -264,6 +264,7 @@ int sys_setxattr (const char *path, const char *name, const void *value, size_t
int sys_fsetxattr (int filedes, const char *name, const void *value, size_t size, int flags);
uint32_t unix_dev_major(SMB_DEV_T dev);
uint32_t unix_dev_minor(SMB_DEV_T dev);
+char *sys_realpath(const char *path);
#if 0
int sys_get_number_of_cores(void);
#endif
diff --git a/source3/lib/system.c b/source3/lib/system.c
index e54b946..0351e37 100644
--- a/source3/lib/system.c
+++ b/source3/lib/system.c
@@ -1236,6 +1236,31 @@ uint32_t unix_dev_minor(SMB_DEV_T dev)
#endif
}
+/**************************************************************************
+ Wrapper for realpath.
+****************************************************************************/
+
+char *sys_realpath(const char *path)
+{
+ char *result;
+
+#ifdef REALPATH_TAKES_NULL
+ result = realpath(path, NULL);
+#else
+ result = SMB_MALLOC_ARRAY(char, PATH_MAX + 1);
+ if (result) {
+ char *resolved_path = realpath(path, result);
+ if (!resolved_path) {
+ SAFE_FREE(result);
+ } else {
+ /* SMB_ASSERT(result == resolved_path) ? */
+ result = resolved_path;
+ }
+ }
+#endif
+ return result;
+}
+
#if 0
/*******************************************************************
Return the number of CPUs.
diff --git a/source3/modules/vfs_default.c b/source3/modules/vfs_default.c
index 819a1a1..f8a70ba 100644
--- a/source3/modules/vfs_default.c
+++ b/source3/modules/vfs_default.c
@@ -2141,20 +2141,7 @@ static char *vfswrap_realpath(vfs_handle_struct *handle, const char *path)
char *result;
START_PROFILE(syscall_realpath);
-#ifdef REALPATH_TAKES_NULL
- result = realpath(path, NULL);
-#else
- result = SMB_MALLOC_ARRAY(char, PATH_MAX+1);
- if (result) {
- char *resolved_path = realpath(path, result);
- if (!resolved_path) {
- SAFE_FREE(result);
- } else {
- /* SMB_ASSERT(result == resolved_path) ? */
- result = resolved_path;
- }
- }
-#endif
+ result = sys_realpath(path);
END_PROFILE(syscall_realpath);
return result;
}
--
2.4.3
From 22d6732e70aef27fa69474ef35a75e07461e5a92 Mon Sep 17 00:00:00 2001
From: Uri Simchoni <uri at samba.org>
Date: Mon, 18 Jan 2016 23:34:48 +0200
Subject: [PATCH 2/2] smbd quotas: avoid stat of foreign file systems
When determining the block device of our file system, avoid
stat'ing paths which are definitely not the mount point of
our file system. This is done to avoid stalling smbd due to
unresponsive network file systems (e.g. NFS) which are not
related to the SMB shares.
See discussion in samba-technical for vfs_fileid:
https://lists.samba.org/archive/samba-technical/2016-January/111553.html
Signed-off-by: Uri Simchoni <uri at samba.org>
---
source3/lib/sysquotas.c | 71 ++++++++++++++++++++++++++++++++++++++++++-------
1 file changed, 61 insertions(+), 10 deletions(-)
diff --git a/source3/lib/sysquotas.c b/source3/lib/sysquotas.c
index 2ef1d1b..bacc4b2 100644
--- a/source3/lib/sysquotas.c
+++ b/source3/lib/sysquotas.c
@@ -40,35 +40,84 @@
#endif /* NO_QUOTACTL_USED */
-#ifdef HAVE_MNTENT
+#if defined(HAVE_MNTENT) && defined(HAVE_REALPATH)
static int sys_path_to_bdev(const char *path, char **mntpath, char **bdev, char **fs)
{
int ret = -1;
SMB_STRUCT_STAT S;
FILE *fp;
- struct mntent *mnt;
+ struct mntent *mnt = NULL;
SMB_DEV_T devno;
+ char *stat_mntpath = NULL;
+ char *p;
/* find the block device file */
-
- if (!path||!mntpath||!bdev||!fs)
- smb_panic("sys_path_to_bdev: called with NULL pointer");
-
(*mntpath) = NULL;
(*bdev) = NULL;
(*fs) = NULL;
-
- if ( sys_stat(path, &S, false) == -1 )
- return (-1);
+
+ if (sys_stat(path, &S, false) != 0) {
+ return -1;
+ }
devno = S.st_ex_dev ;
+ stat_mntpath = sys_realpath(path);
+ if (stat_mntpath == NULL) {
+ DBG_WARNING("realpath(%s) failed - %s\n", path,
+ strerror(errno));
+ goto out;
+ }
+
+ if (sys_stat(stat_mntpath, &S, false) != 0) {
+ DBG_WARNING("cannot stat real path %s - %s\n", stat_mntpath,
+ strerror(errno));
+ goto out;
+ }
+
+ if (S.st_ex_dev != devno) {
+ DBG_WARNING("device on real path has changed\n");
+ goto out;
+ }
+
+ while (true) {
+ p = strrchr(stat_mntpath, '/');
+ if (p == NULL) {
+ DBG_ERR("realpath for %s does not begin with a '/'\n",
+ path);
+ goto out;
+ }
+
+ if (p == stat_mntpath) {
+ ++p;
+ }
+
+ *p = 0;
+ if (sys_stat(stat_mntpath, &S, false) != 0) {
+ DBG_WARNING("cannot stat real path component %s - %s\n",
+ stat_mntpath, strerror(errno));
+ goto out;
+ }
+ if (S.st_ex_dev != devno) {
+ *p = '/';
+ break;
+ }
+
+ if (p <= stat_mntpath + 1) {
+ break;
+ }
+ }
+
fp = setmntent(MOUNTED,"r");
if (fp == NULL) {
- return -1;
+ goto out;
}
while ((mnt = getmntent(fp))) {
+ if (!strequal(mnt->mnt_dir, stat_mntpath)) {
+ continue;
+ }
+
if ( sys_stat(mnt->mnt_dir, &S, false) == -1 )
continue ;
@@ -91,6 +140,8 @@ static int sys_path_to_bdev(const char *path, char **mntpath, char **bdev, char
endmntent(fp) ;
+out:
+ SAFE_FREE(stat_mntpath);
return ret;
}
/* #endif HAVE_MNTENT */
--
2.4.3
More information about the samba-technical
mailing list