]> git.ipfire.org Git - thirdparty/xfsprogs-dev.git/commitdiff
xfs_scrub: warn about suspicious characters in directory/xattr names
authorDarrick J. Wong <darrick.wong@oracle.com>
Fri, 2 Feb 2018 15:32:46 +0000 (09:32 -0600)
committerEric Sandeen <sandeen@redhat.com>
Fri, 2 Feb 2018 15:32:46 +0000 (09:32 -0600)
Look for control characters and punctuation that interfere with shell
globbing in directory entry names and extended attribute key names.
Technically these aren't filesystem corruptions because names are
arbitrary sequences of bytes, but they've been known to cause problems
in the Unix environment so warn if we see them.

Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com>
Reviewed-by: Eric Sandeen <sandeen@redhat.com>
Signed-off-by: Eric Sandeen <sandeen@sandeen.net>
configure.ac
debian/control
include/builddefs.in
m4/Makefile
m4/package_attr.m4 [new file with mode: 0644]
scrub/Makefile
scrub/common.c
scrub/common.h
scrub/phase5.c
scrub/xfs_scrub.h

index 796a91b6805e0f75051509fe07b6af0a9fa6ba65..e2e3f6699b03e1e585ea0fe7f6ddebf0e2b7a3e7 100644 (file)
@@ -166,6 +166,8 @@ AC_HAVE_STATFS_FLAGS
 AC_HAVE_MAP_SYNC
 AC_HAVE_DEVMAPPER
 AC_HAVE_MALLINFO
+AC_PACKAGE_WANT_ATTRIBUTES_H
+AC_HAVE_LIBATTR
 
 if test "$enable_blkid" = yes; then
 AC_HAVE_BLKID_TOPO
index f5980b26fd9ade64e79f07b0cdf59881b22e10b0..f664a6b669be0947ef73f1f913b7a883010007bb 100644 (file)
@@ -3,7 +3,7 @@ Section: admin
 Priority: optional
 Maintainer: XFS Development Team <linux-xfs@vger.kernel.org>
 Uploaders: Nathan Scott <nathans@debian.org>, Anibal Monsalve Salazar <anibal@debian.org>
-Build-Depends: uuid-dev, dh-autoreconf, debhelper (>= 5), gettext, libtool, libreadline-gplv2-dev | libreadline5-dev, libblkid-dev (>= 2.17), linux-libc-dev, libdevmapper-dev
+Build-Depends: uuid-dev, dh-autoreconf, debhelper (>= 5), gettext, libtool, libreadline-gplv2-dev | libreadline5-dev, libblkid-dev (>= 2.17), linux-libc-dev, libdevmapper-dev, libattr1-dev
 Standards-Version: 3.9.1
 Homepage: https://xfs.wiki.kernel.org/
 
index 28cf0d889d1a621fe7966333e5f838fa07dd2a0b..cc1b7e2e738ba2c42f8e567ee2fc1b7152a31d28 100644 (file)
@@ -120,6 +120,7 @@ HAVE_STATFS_FLAGS = @have_statfs_flags@
 HAVE_MAP_SYNC = @have_map_sync@
 HAVE_DEVMAPPER = @have_devmapper@
 HAVE_MALLINFO = @have_mallinfo@
+HAVE_LIBATTR = @have_libattr@
 
 GCCFLAGS = -funsigned-char -fno-strict-aliasing -Wall
 #         -Wbitwise -Wno-transparent-union -Wno-old-initializer -Wno-decl
index 77f2edda9da3bc72ec5c5216f25342de5ebc079a..d5f1d2fc1da7f66c429f8bfaf84e8236f095acd1 100644 (file)
@@ -17,6 +17,7 @@ LSRCFILES = \
        package_blkid.m4 \
        package_devmapper.m4 \
        package_globals.m4 \
+       package_attr.m4 \
        package_libcdev.m4 \
        package_pthread.m4 \
        package_sanitizer.m4 \
diff --git a/m4/package_attr.m4 b/m4/package_attr.m4
new file mode 100644 (file)
index 0000000..4324923
--- /dev/null
@@ -0,0 +1,23 @@
+AC_DEFUN([AC_PACKAGE_WANT_ATTRIBUTES_H],
+  [
+    AC_CHECK_HEADERS(attr/attributes.h)
+  ])
+
+#
+# Check if we have a ATTR_ROOT flag and libattr structures
+#
+AC_DEFUN([AC_HAVE_LIBATTR],
+  [ AC_MSG_CHECKING([for struct attrlist_cursor])
+    AC_TRY_COMPILE([
+#include <sys/types.h>
+#include <attr/attributes.h>
+       ], [
+struct attrlist_cursor *cur;
+struct attrlist *list;
+struct attrlist_ent *ent;
+int flags = ATTR_ROOT;
+       ], have_libattr=yes
+          AC_MSG_RESULT(yes),
+          AC_MSG_RESULT(no))
+    AC_SUBST(have_libattr)
+  ])
index adb868ebb38890c805abb6a0527d652be6bb22e4..67ac6afa4d5f10598f87b428fbe3c8550487bef2 100644 (file)
@@ -53,8 +53,14 @@ ifeq ($(HAVE_SYNCFS),yes)
 LCFLAGS += -DHAVE_SYNCFS
 endif
 
+ifeq ($(HAVE_LIBATTR),yes)
+LCFLAGS += -DHAVE_LIBATTR
+endif
+
 default: depend $(LTCOMMAND)
 
+phase5.o: $(TOPDIR)/include/builddefs
+
 include $(BUILDRULES)
 
 install: default $(INSTALL_SCRUB)
index 81869e48bf78e7a0ef19f7869b5332413318c2cf..5e2d7c603a16667ca53fe9ebf768c24adc50663e 100644 (file)
@@ -285,3 +285,57 @@ background_sleep(void)
        tv.tv_nsec = time % 1000000;
        nanosleep(&tv, NULL);
 }
+
+/*
+ * Return the input string with non-printing bytes escaped.
+ * Caller must free the buffer.
+ */
+char *
+string_escape(
+       const char              *in)
+{
+       char                    *str;
+       const char              *p;
+       char                    *q;
+       int                     x;
+
+       str = malloc(strlen(in) * 4);
+       if (!str)
+               return NULL;
+       for (p = in, q = str; *p != '\0'; p++) {
+               if (isprint(*p)) {
+                       *q = *p;
+                       q++;
+               } else {
+                       x = sprintf(q, "\\x%02x", *p);
+                       q += x;
+               }
+       }
+       *q = '\0';
+       return str;
+}
+
+/*
+ * Record another naming warning, and decide if it's worth
+ * complaining about.
+ */
+bool
+should_warn_about_name(
+       struct scrub_ctx        *ctx)
+{
+       bool                    whine;
+       bool                    res;
+
+       pthread_mutex_lock(&ctx->lock);
+       ctx->naming_warnings++;
+       whine = ctx->naming_warnings == TOO_MANY_NAME_WARNINGS;
+       res = ctx->naming_warnings < TOO_MANY_NAME_WARNINGS;
+       pthread_mutex_unlock(&ctx->lock);
+
+       if (whine && !(debug || verbose))
+               str_info(ctx, ctx->mntpoint,
+_("More than %u naming warnings, shutting up."),
+                               TOO_MANY_NAME_WARNINGS);
+
+       return debug || verbose || res;
+}
index 96fc8708f95b3273720ae6db8e955c06b2221fb6..336fc408aa81662e50b4d317b0f7267705ed17bb 100644 (file)
@@ -74,5 +74,9 @@ static inline int syncfs(int fd)
 
 bool find_mountpoint(char *mtab, struct scrub_ctx *ctx);
 void background_sleep(void);
+char *string_escape(const char *in);
+
+#define TOO_MANY_NAME_WARNINGS 10000
+bool should_warn_about_name(struct scrub_ctx *ctx);
 
 #endif /* XFS_SCRUB_COMMON_H_ */
index 0b161e3187defa233d65419ebd967a67c273bb16..98d30f87350b26805517afebed541d674585594f 100644 (file)
 #include <stdio.h>
 #include <stdint.h>
 #include <stdbool.h>
+#include <dirent.h>
 #include <sys/types.h>
 #include <sys/stat.h>
 #include <sys/statvfs.h>
+#ifdef HAVE_LIBATTR
+# include <attr/attributes.h>
+#endif
 #include "xfs.h"
+#include "handle.h"
 #include "path.h"
 #include "workqueue.h"
 #include "xfs_scrub.h"
 
 /* Phase 5: Check directory connectivity. */
 
+/*
+ * Warn about problematic bytes in a directory/attribute name.  That means
+ * terminal control characters and escape sequences, since that could be used
+ * to do something naughty to the user's computer and/or break scripts.  XFS
+ * doesn't consider any byte sequence invalid, so don't flag these as errors.
+ */
+static bool
+xfs_scrub_check_name(
+       struct scrub_ctx        *ctx,
+       const char              *descr,
+       const char              *namedescr,
+       const char              *name)
+{
+       const char              *p;
+       bool                    bad = false;
+       char                    *errname;
+
+       /* Complain about zero length names. */
+       if (*name == '\0' && should_warn_about_name(ctx)) {
+               str_warn(ctx, descr, _("Zero length name found."));
+               return true;
+       }
+
+       /* control characters */
+       for (p = name; *p; p++) {
+               if ((*p >= 1 && *p <= 31) || *p == 127) {
+                       bad = true;
+                       break;
+               }
+       }
+
+       if (bad && should_warn_about_name(ctx)) {
+               errname = string_escape(name);
+               if (!errname) {
+                       str_errno(ctx, descr);
+                       return false;
+               }
+               str_info(ctx, descr,
+_("Control character found in %s name \"%s\"."),
+                               namedescr, errname);
+               free(errname);
+       }
+
+       return true;
+}
+
+/*
+ * Iterate a directory looking for filenames with problematic
+ * characters.
+ */
+static bool
+xfs_scrub_scan_dirents(
+       struct scrub_ctx        *ctx,
+       const char              *descr,
+       int                     *fd)
+{
+       DIR                     *dir;
+       struct dirent           *dentry;
+       bool                    moveon = true;
+
+       dir = fdopendir(*fd);
+       if (!dir) {
+               str_errno(ctx, descr);
+               goto out;
+       }
+       *fd = -1; /* closedir will close *fd for us */
+
+       dentry = readdir(dir);
+       while (dentry) {
+               moveon = xfs_scrub_check_name(ctx, descr, _("directory"),
+                               dentry->d_name);
+               if (!moveon)
+                       break;
+               dentry = readdir(dir);
+       }
+
+       closedir(dir);
+out:
+       return moveon;
+}
+
+#ifdef HAVE_LIBATTR
+/* Routines to scan all of an inode's xattrs for name problems. */
+struct xfs_attr_ns {
+       int                     flags;
+       const char              *name;
+};
+
+static const struct xfs_attr_ns attr_ns[] = {
+       {0,                     "user"},
+       {ATTR_ROOT,             "system"},
+       {ATTR_SECURE,           "secure"},
+       {0, NULL},
+};
+
+/*
+ * Check all the xattr names in a particular namespace of a file handle
+ * for Unicode normalization problems or collisions.
+ */
+static bool
+xfs_scrub_scan_fhandle_namespace_xattrs(
+       struct scrub_ctx                *ctx,
+       const char                      *descr,
+       struct xfs_handle               *handle,
+       const struct xfs_attr_ns        *attr_ns)
+{
+       struct attrlist_cursor          cur;
+       char                            attrbuf[XFS_XATTR_LIST_MAX];
+       char                            keybuf[NAME_MAX + 1];
+       struct attrlist                 *attrlist = (struct attrlist *)attrbuf;
+       struct attrlist_ent             *ent;
+       bool                            moveon = true;
+       int                             i;
+       int                             error;
+
+       memset(attrbuf, 0, XFS_XATTR_LIST_MAX);
+       memset(&cur, 0, sizeof(cur));
+       memset(keybuf, 0, NAME_MAX + 1);
+       error = attr_list_by_handle(handle, sizeof(*handle), attrbuf,
+                       XFS_XATTR_LIST_MAX, attr_ns->flags, &cur);
+       while (!error) {
+               /* Examine the xattrs. */
+               for (i = 0; i < attrlist->al_count; i++) {
+                       ent = ATTR_ENTRY(attrlist, i);
+                       snprintf(keybuf, NAME_MAX, "%s.%s", attr_ns->name,
+                                       ent->a_name);
+                       moveon = xfs_scrub_check_name(ctx, descr,
+                                       _("extended attribute"), keybuf);
+                       if (!moveon)
+                               goto out;
+               }
+
+               if (!attrlist->al_more)
+                       break;
+               error = attr_list_by_handle(handle, sizeof(*handle), attrbuf,
+                               XFS_XATTR_LIST_MAX, attr_ns->flags, &cur);
+       }
+       if (error && errno != ESTALE)
+               str_errno(ctx, descr);
+out:
+       return moveon;
+}
+
+/*
+ * Check all the xattr names in all the xattr namespaces for problematic
+ * characters.
+ */
+static bool
+xfs_scrub_scan_fhandle_xattrs(
+       struct scrub_ctx                *ctx,
+       const char                      *descr,
+       struct xfs_handle               *handle)
+{
+       const struct xfs_attr_ns        *ns;
+       bool                            moveon = true;
+
+       for (ns = attr_ns; ns->name; ns++) {
+               moveon = xfs_scrub_scan_fhandle_namespace_xattrs(ctx, descr,
+                               handle, ns);
+               if (!moveon)
+                       break;
+       }
+       return moveon;
+}
+#else
+static inline bool
+xfs_scrub_scan_fhandle_xattrs(
+       struct scrub_ctx        *ctx,
+       const char              *descr,
+       struct xfs_handle       *handle)
+{
+       return true;
+}
+#endif /* HAVE_LIBATTR */
+
 /*
  * Verify the connectivity of the directory tree.
  * We know that the kernel's open-by-handle function will try to reconnect
@@ -58,6 +238,11 @@ xfs_scrub_connections(
                        (uint64_t)bstat->bs_ino, agno, agino);
        background_sleep();
 
+        /* Warn about naming problems in xattrs. */
+        moveon = xfs_scrub_scan_fhandle_xattrs(ctx, descr, handle);
+        if (!moveon)
+                goto out;
+
        /* Open the dir, let the kernel try to reconnect it to the root. */
        if (S_ISDIR(bstat->bs_mode)) {
                fd = xfs_open_handle(handle);
@@ -69,6 +254,13 @@ xfs_scrub_connections(
                }
        }
 
+        /* Warn about naming problems in the directory entries. */
+        if (fd >= 0 && S_ISDIR(bstat->bs_mode)) {
+                moveon = xfs_scrub_scan_dirents(ctx, descr, &fd);
+                if (!moveon)
+                        goto out;
+        }
+
 out:
        if (fd >= 0)
                close(fd);
index fd7d7aa0729ebb75df35ec043ba3927387061d31..0aef76b9304b72e8eccdcd5efc9d098d2cb49ea1 100644 (file)
@@ -87,6 +87,7 @@ struct scrub_ctx {
        unsigned long long      errors_found;
        unsigned long long      warnings_found;
        unsigned long long      inodes_checked;
+       unsigned long long      naming_warnings;
        bool                    need_repair;
        bool                    preen_triggers[XFS_SCRUB_TYPE_NR];
 };