| Message ID | 8d4e11f95111306427e18d08d4ba9ea759c24576.1468394884.git.laurent.georget@supelec.fr (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
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
>> >> + 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
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;
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(-)