[SSSD] [PATCH] Do not copy special files when creating homedir

Jakub Hrozek jhrozek at redhat.com
Tue Jul 9 21:45:24 UTC 2013


On Thu, Jun 27, 2013 at 07:01:33PM +0200, Jakub Hrozek wrote:
> On Thu, Jun 27, 2013 at 02:02:44PM +0200, Pavel Březina wrote:
> > On 06/27/2013 01:09 PM, Ondrej Kos wrote:
> > >On 06/27/2013 12:48 PM, Pavel Březina wrote:
> > >>On 06/26/2013 03:16 PM, Ondrej Kos wrote:
> > >>>Hi,
> > >>>
> > >>>Attached find a patch addressing sssd trac issue
> > >>>https://fedorahosted.org/sssd/ticket/1778
> > >>>
> > >>>Ondra
> > >>
> > >>The patch works as expected, I have just a comment to debug message.
> > >>
> > >>>+        /* Is a special file */
> > >>>+        DEBUG(SSSDBG_FUNC_DATA, ("Cannot copy '%s', it is a special
> > >>>file.\n",
> > >>>+                  src_ent_path));
> > >>
> > >>"Cannot copy" looks like this is something unexpected, an error. I'd
> > >>recommend to use something like "'%s' is a special file, skipping...".
> > >>
> > >
> > >Thanks for the review, new patch is attached.
> > >
> > >Ondra
> > 
> > Ack.
> 
> Nack for 1.10, we can't include man page changes at this point, we're string
> frozen. When 1.11 branches off (probably tomorrow), I'll be able to push
> this patch to 1.11 but for inclusion in 1.10 please resend without the
> man page change.

However, ack for 1.11 (master). Also attached is the same patch just
with the manpage hunk removed. I think it is good to go for 1.10 but I'd
like someone to ack it for sanity reasons.
-------------- next part --------------
>From de6b1557180e0c36ba2655e6f8a6bc6510559942 Mon Sep 17 00:00:00 2001
From: Ondrej Kos <okos at redhat.com>
Date: Tue, 9 Jul 2013 23:38:42 +0200
Subject: [PATCH] Do not copy special files when creating homedir

---
 src/tests/files-tests.c |  8 +++---
 src/tools/files.c       | 71 +++----------------------------------------------
 2 files changed, 6 insertions(+), 73 deletions(-)

diff --git a/src/tests/files-tests.c b/src/tests/files-tests.c
index 4225098c27e6b1492aeb2985eb821fe8ba05a4b9..5851a721ff1351f80890b42334a0bb734a74ffe5 100644
--- a/src/tests/files-tests.c
+++ b/src/tests/files-tests.c
@@ -245,7 +245,6 @@ START_TEST(test_copy_node)
     int ret;
     char origpath[PATH_MAX+1];
     char *tmp;
-    struct stat statbuf;
 
     errno = 0;
     fail_unless(getcwd(origpath, PATH_MAX) == origpath, "Cannot getcwd\n");
@@ -267,14 +266,13 @@ START_TEST(test_copy_node)
     ret = copy_tree(dir_path, dst_path, 0700, uid, gid);
     fail_unless(ret == EOK, "copy_tree failed\n");
 
-    /* check if really copied */
+    /* check if really copied and without special files */
     ret = access(dst_path, F_OK);
     fail_unless(ret == 0, "destination directory not there\n");
 
     tmp = talloc_asprintf(test_ctx, "%s/testnode", dst_path);
-    ret = lstat(tmp, &statbuf);
-    fail_unless(ret == 0, "cannot stat the node %s\n", tmp);
-    fail_unless(S_ISFIFO(statbuf.st_mode), "%s not a char device??\n", tmp);
+    ret = access(tmp, F_OK);
+    fail_unless(ret == -1, "special file %s exists, it shouldn't\n", tmp);
     talloc_free(tmp);
 }
 END_TEST
diff --git a/src/tools/files.c b/src/tools/files.c
index 1c1b0855869d784959d274e5bd52b930d0d51887..ff04d09f8ba14802098b8177dc7c905fa75f4e80 100644
--- a/src/tools/files.c
+++ b/src/tools/files.c
@@ -351,66 +351,6 @@ copy_symlink(int src_dir_fd,
     return EOK;
 }
 
-/* Create a special file named file_name under a directory with file
- * descriptor dst_dir_fd. full_path is used for both setting SELinux
- * context and logging. The node is owned by uid/gid and its mode
- * and device number is read from statp.
- */
-static int copy_special(int dst_dir_fd,
-                        const char *file_name,
-                        const char *full_path,
-                        const struct stat *statp,
-                        uid_t uid, gid_t gid)
-{
-    int ret;
-
-    ret = selinux_file_context(full_path);
-    if (ret != 0) {
-        DEBUG(SSSDBG_MINOR_FAILURE,
-              ("Failed to set SELinux context for [%s]\n", full_path));
-        /* Not fatal */
-    }
-
-    ret = mknodat(dst_dir_fd, file_name, statp->st_mode & ~07777,
-                  statp->st_rdev);
-    if (ret != 0) {
-        ret = errno;
-        DEBUG(SSSDBG_OP_FAILURE,
-              ("Cannot mknod special file '%s': [%d][%s].\n",
-              full_path, ret, strerror(ret)));
-        return ret;
-    }
-
-    ret = fchownat(dst_dir_fd, file_name, uid, gid, 0);
-    if (ret != 0) {
-        ret = errno;
-        DEBUG(SSSDBG_CRIT_FAILURE,
-              ("fchownat failed for '%s': [%d][%s]\n",
-              full_path, ret, strerror(ret)));
-        return ret;
-    }
-
-    ret = fchmodat(dst_dir_fd, file_name, statp->st_mode & 07777, 0);
-    if (ret != 0) {
-        ret = errno;
-        DEBUG(SSSDBG_CRIT_FAILURE,
-              ("fchmodat failed for '%s': [%d][%s]\n",
-              full_path, ret, strerror(ret)));
-        return ret;
-    }
-
-    ret = sss_timeat_set(dst_dir_fd, file_name, statp, 0);
-    if (ret == -1) {
-        ret = errno;
-        DEBUG(SSSDBG_MINOR_FAILURE,
-              ("utimensat failed for '%s': [%d][%s]\n",
-              full_path, ret, strerror(ret)));
-        /* Do not fail, this shouldn't be fatal */
-    }
-
-    return EOK;
-}
-
 /* Copy bytes from input file descriptor ifd into file named
  * dst_named under directory with dest_dir_fd. Own the new file
  * by uid/gid
@@ -602,14 +542,9 @@ copy_entry(struct copy_ctx *cctx,
             goto done;
         }
     } else {
-        /* Copy a special file */
-        ret = copy_special(dest_dir_fd, ent_name, dest_ent_path,
-                           &st, cctx->uid, cctx->gid);
-        if (ret) {
-            DEBUG(SSSDBG_OP_FAILURE, ("Cannot copy '%s' to '%s'\n",
-                  src_ent_path, dest_ent_path));
-            goto done;
-        }
+        /* Is a special file */
+        DEBUG(SSSDBG_FUNC_DATA, ("'%s' is a special file, skipping.\n",
+                  src_ent_path));
     }
 
     ret = EOK;
-- 
1.8.3.1



More information about the sssd-devel mailing list