[1/2] Add LSM hooks in vmsplice, splice and tee
diff mbox

Message ID 8d4e11f95111306427e18d08d4ba9ea759c24576.1468394884.git.laurent.georget@supelec.fr
State New
Headers show

Commit Message

Laurent Georget July 13, 2016, 7:32 a.m. UTC
Signed-off-by: Laurent Georget <laurent.georget@supelec.fr>

---
 fs/splice.c              | 29 +++++++++++++++++++++++------
 include/linux/security.h |  7 +++++++
 2 files changed, 30 insertions(+), 6 deletions(-)

Comments

Stephen Smalley July 21, 2016, 1:34 p.m. UTC | #1
On 07/13/2016 03:32 AM, Laurent Georget wrote:
> Signed-off-by: Laurent Georget <laurent.georget@supelec.fr>
> 
> ---
>  fs/splice.c              | 29 +++++++++++++++++++++++------
>  include/linux/security.h |  7 +++++++
>  2 files changed, 30 insertions(+), 6 deletions(-)
> 
> diff --git a/fs/splice.c b/fs/splice.c
> index dd9bf7e..308514e 100644
> --- a/fs/splice.c
> +++ b/fs/splice.c
> @@ -32,6 +32,7 @@
>  #include <linux/gfp.h>
>  #include <linux/socket.h>
>  #include <linux/compat.h>
> +#include <linux/security.h>
>  #include "internal.h"
>  
>  /*
> @@ -1375,6 +1376,10 @@ static long do_splice(struct file *in, loff_t __user *off_in,
>  		if (ipipe == opipe)
>  			return -EINVAL;
>  
> +		ret = security_file_splice_pipe_to_pipe(in, out);
> +		if (ret)
> +			return ret;
> +

Do we want a new hook here or just two calls to security_file_permission()?

>  		return splice_pipe_to_pipe(ipipe, opipe, len, flags);
>  	}
>  
> @@ -1564,6 +1569,10 @@ static long vmsplice_to_user(struct file *file, const struct iovec __user *uiov,
>  	if (!pipe)
>  		return -EBADF;
>  
> +	ret = security_file_permission(file, MAY_READ);
> +	if (ret)
> +		return ret;
> +
>  	ret = import_iovec(READ, uiov, nr_segs,
>  			   ARRAY_SIZE(iovstack), &iov, &iter);
>  	if (ret < 0)
> @@ -1610,6 +1619,10 @@ static long vmsplice_to_pipe(struct file *file, const struct iovec __user *iov,
>  	if (!pipe)
>  		return -EBADF;
>  
> +	ret = security_file_permission(file, MAY_WRITE);
> +	if (ret)
> +		return ret;
> +
>  	if (splice_grow_spd(pipe, &spd))
>  		return -ENOMEM;
>  
> @@ -2005,18 +2018,22 @@ static long do_tee(struct file *in, struct file *out, size_t len,
>  	 * copying the data.
>  	 */
>  	if (ipipe && opipe && ipipe != opipe) {
> +		ret = security_file_splice_pipe_to_pipe(in, out);
> +		if (ret)
> +			goto out;
>  		/*
>  		 * Keep going, unless we encounter an error. The ipipe/opipe
>  		 * ordering doesn't really matter.
>  		 */
>  		ret = ipipe_prep(ipipe, flags);
> -		if (!ret) {
> -			ret = opipe_prep(opipe, flags);
> -			if (!ret)
> -				ret = link_pipe(ipipe, opipe, len, flags);
> -		}
> +		if (ret)
> +			goto out;
> +		ret = opipe_prep(opipe, flags);
> +		if (ret)
> +			goto out;
> +		ret = link_pipe(ipipe, opipe, len, flags);
>  	}
> -
> +out:
>  	return ret;
>  }
>  
> diff --git a/include/linux/security.h b/include/linux/security.h
> index 157f0cb..c3e2109 100644
> --- a/include/linux/security.h
> +++ b/include/linux/security.h
> @@ -292,6 +292,7 @@ int security_file_send_sigiotask(struct task_struct *tsk,
>  				 struct fown_struct *fown, int sig);
>  int security_file_receive(struct file *file);
>  int security_file_open(struct file *file, const struct cred *cred);
> +int security_file_splice_pipe_to_pipe(struct file *in, struct file *out);
>  int security_task_create(unsigned long clone_flags);
>  void security_task_free(struct task_struct *task);
>  int security_cred_alloc_blank(struct cred *cred, gfp_t gfp);
> @@ -815,6 +816,12 @@ static inline int security_file_open(struct file *file,
>  	return 0;
>  }
>  
> +static inline int security_file_splice_pipe_to_pipe(struct file *in,
> +						    struct file *out)
> +{
> +	return 0;
> +}
> +

If adding a new hook, you need more than just this for the
CONFIG_SECURITY=y case.

>  static inline int security_task_create(unsigned long clone_flags)
>  {
>  	return 0;
> 

--
To unsubscribe from this list: send the line "unsubscribe linux-security-module" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Laurent Georget July 21, 2016, 1:59 p.m. UTC | #2
>>  
>> +		ret = security_file_splice_pipe_to_pipe(in, out);
>> +		if (ret)
>> +			return ret;
>> +
> 
> Do we want a new hook here or just two calls to security_file_permission()?

This is something I've been thinking about. Maybe a new hook whose default implementation
is calling security_file_permission() twice... But that is arguably over-engineered.
And calling security_file_permission() would be more consistent with what I do in the cases
"file to pipe" and "pipe to file".

> 
> If adding a new hook, you need more than just this for the
> CONFIG_SECURITY=y case.
> 

Uh... yes of course, the call_int_hook() in security.c is missing in this patch.

Anyway, I've taken note of the comment Casey Schaufler and you have made about not proposing
new hooks without a use case. So, I'll reengineer this patch series with an implementation for
the new hooks in SELinux and post it again.

Thank you

Laurent Georget

Patch
diff mbox

diff --git a/fs/splice.c b/fs/splice.c
index dd9bf7e..308514e 100644
--- a/fs/splice.c
+++ b/fs/splice.c
@@ -32,6 +32,7 @@ 
 #include <linux/gfp.h>
 #include <linux/socket.h>
 #include <linux/compat.h>
+#include <linux/security.h>
 #include "internal.h"
 
 /*
@@ -1375,6 +1376,10 @@  static long do_splice(struct file *in, loff_t __user *off_in,
 		if (ipipe == opipe)
 			return -EINVAL;
 
+		ret = security_file_splice_pipe_to_pipe(in, out);
+		if (ret)
+			return ret;
+
 		return splice_pipe_to_pipe(ipipe, opipe, len, flags);
 	}
 
@@ -1564,6 +1569,10 @@  static long vmsplice_to_user(struct file *file, const struct iovec __user *uiov,
 	if (!pipe)
 		return -EBADF;
 
+	ret = security_file_permission(file, MAY_READ);
+	if (ret)
+		return ret;
+
 	ret = import_iovec(READ, uiov, nr_segs,
 			   ARRAY_SIZE(iovstack), &iov, &iter);
 	if (ret < 0)
@@ -1610,6 +1619,10 @@  static long vmsplice_to_pipe(struct file *file, const struct iovec __user *iov,
 	if (!pipe)
 		return -EBADF;
 
+	ret = security_file_permission(file, MAY_WRITE);
+	if (ret)
+		return ret;
+
 	if (splice_grow_spd(pipe, &spd))
 		return -ENOMEM;
 
@@ -2005,18 +2018,22 @@  static long do_tee(struct file *in, struct file *out, size_t len,
 	 * copying the data.
 	 */
 	if (ipipe && opipe && ipipe != opipe) {
+		ret = security_file_splice_pipe_to_pipe(in, out);
+		if (ret)
+			goto out;
 		/*
 		 * Keep going, unless we encounter an error. The ipipe/opipe
 		 * ordering doesn't really matter.
 		 */
 		ret = ipipe_prep(ipipe, flags);
-		if (!ret) {
-			ret = opipe_prep(opipe, flags);
-			if (!ret)
-				ret = link_pipe(ipipe, opipe, len, flags);
-		}
+		if (ret)
+			goto out;
+		ret = opipe_prep(opipe, flags);
+		if (ret)
+			goto out;
+		ret = link_pipe(ipipe, opipe, len, flags);
 	}
-
+out:
 	return ret;
 }
 
diff --git a/include/linux/security.h b/include/linux/security.h
index 157f0cb..c3e2109 100644
--- a/include/linux/security.h
+++ b/include/linux/security.h
@@ -292,6 +292,7 @@  int security_file_send_sigiotask(struct task_struct *tsk,
 				 struct fown_struct *fown, int sig);
 int security_file_receive(struct file *file);
 int security_file_open(struct file *file, const struct cred *cred);
+int security_file_splice_pipe_to_pipe(struct file *in, struct file *out);
 int security_task_create(unsigned long clone_flags);
 void security_task_free(struct task_struct *task);
 int security_cred_alloc_blank(struct cred *cred, gfp_t gfp);
@@ -815,6 +816,12 @@  static inline int security_file_open(struct file *file,
 	return 0;
 }
 
+static inline int security_file_splice_pipe_to_pipe(struct file *in,
+						    struct file *out)
+{
+	return 0;
+}
+
 static inline int security_task_create(unsigned long clone_flags)
 {
 	return 0;