Re: [PATCH v15 04/13] task_isolation: add initial support

[Eugene Syromiatnikov <esyr@redhat.com>]

>  	case PR_GET_FP_MODE:
>  		error = GET_FP_MODE(me);
>  		break;
> +#ifdef CONFIG_TASK_ISOLATION
> +	case PR_SET_TASK_ISOLATION:
> +		error = task_isolation_set(arg2);
> +		break;
> +	case PR_GET_TASK_ISOLATION:
> +		error = me->task_isolation_flags;
> +		break;
> +#endif
>  	default:
>  		error = -EINVAL;
>  		break;

It is not a very good idea to ignore the values of unused arguments; it
prevents future their usage, as user space can pass some garbage values
here. Check out the code for newer prctl handlers, like
PR_SET_NO_NEW_PRIVS, PR_SET_THP_DISABLE, or PR_MPX_ENABLE_MANAGEMENT
(PR_[SG]_FP_MODE is an unfortunate recent omission).

The other thing is the usage of #ifdef's, which is generally avoided
there. Also, the patch for man-pages, describing the new prctl calls, is
missing.

Please take a look at the following patch, which is made in an
attempt to fix aforementioned issues (it is assumed it can be squashed
with the current one).

---
 include/linux/isolation.h | 12 ++++++++++++
 kernel/sys.c              | 16 ++++++++++++----
 2 files changed, 24 insertions(+), 4 deletions(-)

diff --git a/include/linux/isolation.h b/include/linux/isolation.h
index 02728b1..ae24cad 100644
--- a/include/linux/isolation.h
+++ b/include/linux/isolation.h
@@ -9,6 +9,13 @@
 
 #ifdef CONFIG_TASK_ISOLATION
 
+#ifndef GET_TASK_ISOLATION
+# define GET_TASK_ISOLATION(me)		task_isolation_get(me)
+#endif
+#ifndef SET_TASK_ISOLATION
+# define SET_TASK_ISOLATION(me, a)	task_isolation_set(a)
+#endif
+
 /* cpus that are configured to support task isolation */
 extern cpumask_var_t task_isolation_map;
 
@@ -22,6 +29,11 @@ static inline bool task_isolation_possible(int cpu)
 
 extern int task_isolation_set(unsigned int flags);
 
+static inline unsigned int task_isolation_get(struct task_struct *ts)
+{
+	return ts->task_isolation_flags;
+}
+
 extern bool task_isolation_ready(void);
 extern void task_isolation_enter(void);
 
diff --git a/kernel/sys.c b/kernel/sys.c
index 3de863f..96e0873 100644
--- a/kernel/sys.c
+++ b/kernel/sys.c
@@ -104,6 +104,12 @@
 #ifndef SET_FP_MODE
 # define SET_FP_MODE(a,b)	(-EINVAL)
 #endif
+#ifndef GET_TASK_ISOLATION
+# define GET_TASK_ISOLATION(me)		(-EINVAL)
+#endif
+#ifndef SET_TASK_ISOLATION
+# define SET_TASK_ISOLATION(me, a)	(-EINVAL)
+#endif
 
 /*
  * this is where the system-wide overflow UID and GID are defined, for
@@ -2262,14 +2268,16 @@ SYSCALL_DEFINE5(prctl, int, option, unsigned long, arg2, unsigned long, arg3,
 	case PR_GET_FP_MODE:
 		error = GET_FP_MODE(me);
 		break;
-#ifdef CONFIG_TASK_ISOLATION
 	case PR_SET_TASK_ISOLATION:
-		error = task_isolation_set(arg2);
+		if (arg3 || arg4 || arg5)
+			return -EINVAL;
+		error = SET_TASK_ISOLATION(me, arg2);
 		break;
 	case PR_GET_TASK_ISOLATION:
-		error = me->task_isolation_flags;
+		if (arg2 || arg3 || arg4 || arg5)
+			return -EINVAL;
+		error = GET_TASK_ISOLATION(me);
 		break;
-#endif
 	default:
 		error = -EINVAL;
 		break;
-- 
2.9.3