Re: [PATCH] mk{dir, fifo, nod}: with -Z, create SMACK security context

[Bernhard Voelker]

On 07/02/2013 05:00 AM, Pádraig Brady wrote:
> I noticed a small inconsistency in id -Z.

Thanks, good catch.
What about enhancing the comment above, too?

-   /* If we are on a selinux-enabled kernel, no user is specified, and
+   /* If we are on a SELinux/SMACK-enabled kernel, no user is specified, and
    ...


And a minor nit which I'm not sure about: shouldn't we write SMACK all
upper-case, at least in the commit title?

- Subject: [PATCH 1/2] id: don't show smack errors unless -Z is specified
+ Subject: [PATCH 1/2] id: don't show SMACK errors unless -Z is specified

- Subject: [PATCH 2/2] maint: refactor smack interface to a separate module
+ Subject: [PATCH 2/2] maint: refactor SMACK interface to a separate module


> Also the HAVE_SMACK ifdefs were beginning to proliferate in the code.

In mk{dir,fifo,nod}.c:main, the variable ret and issuing the error message
should be moved into the body of the if (context) block as shown in the
patch below.

Have a nice day,
Berny


diff --git a/src/mkdir.c b/src/mkdir.c
index 479faca..efd3184 100644
--- a/src/mkdir.c
+++ b/src/mkdir.c
@@ -152,7 +152,6 @@ main (int argc, char **argv)
   int optc;
   security_context_t scontext = NULL;
   struct mkdir_options options;
-  int ret = 0;

   options.make_ancestor_function = NULL;
   options.mode = S_IRWXUGO;
@@ -198,16 +197,18 @@ main (int argc, char **argv)

   if (scontext)
     {
+      int ret = 0;
       if (is_smack_enabled ())
         ret = smack_set_label_for_self (scontext);
       else
         ret = setfscreatecon (scontext);
+
+      if (ret < 0)
+        error (EXIT_FAILURE, errno,
+               _("failed to set default file creation context to %s"),
+               quote (scontext));
     }

-  if (ret < 0)
-    error (EXIT_FAILURE, errno,
-           _("failed to set default file creation context to %s"),
-           quote (scontext));

   if (options.make_ancestor_function || specified_mode)
     {
diff --git a/src/mkfifo.c b/src/mkfifo.c
index f9fcc0a..4c6dac4 100644
--- a/src/mkfifo.c
+++ b/src/mkfifo.c
@@ -77,7 +77,6 @@ main (int argc, char **argv)
   int exit_status = EXIT_SUCCESS;
   int optc;
   security_context_t scontext = NULL;
-  int ret = 0;

   initialize_main (&argc, &argv);
   set_program_name (argv[0]);
@@ -112,16 +111,17 @@ main (int argc, char **argv)

   if (scontext)
     {
+      int ret = 0;
       if (is_smack_enabled ())
         ret = smack_set_label_for_self (scontext);
       else
         ret = setfscreatecon (scontext);
-    }

-  if (ret < 0)
-    error (EXIT_FAILURE, errno,
-           _("failed to set default file creation context to %s"),
-           quote (scontext));
+      if (ret < 0)
+        error (EXIT_FAILURE, errno,
+               _("failed to set default file creation context to %s"),
+               quote (scontext));
+    }

   newmode = MODE_RW_UGO;
   if (specified_mode)
diff --git a/src/mknod.c b/src/mknod.c
index 4fd6ed0..c79468c 100644
--- a/src/mknod.c
+++ b/src/mknod.c
@@ -94,7 +94,6 @@ main (int argc, char **argv)
   int expected_operands;
   mode_t node_type;
   security_context_t scontext = NULL;
-  int ret = 0;

   initialize_main (&argc, &argv);
   set_program_name (argv[0]);
@@ -168,16 +167,17 @@ main (int argc, char **argv)

   if (scontext)
     {
+      int ret = 0;
       if (is_smack_enabled ())
         ret = smack_set_label_for_self (scontext);
       else
         ret = setfscreatecon (scontext);
-    }

-  if (ret < 0)
-    error (EXIT_FAILURE, errno,
-           _("failed to set default file creation context to %s"),
-           quote (scontext));
+      if (ret < 0)
+        error (EXIT_FAILURE, errno,
+               _("failed to set default file creation context to %s"),
+               quote (scontext));
+    }

   /* Only check the first character, to allow mnemonic usage like
      'mknod /dev/rst0 character 18 0'. */