Revalidate read/write permissions for splice(2) and vmslice(2), in case
security policy has changed since the files were opened.
Signed-off-by: James Morris <[email protected]>
Acked-by: Stephen Smalley <[email protected]>
---
Please review. Note that this brings the splice code into line with other
mechanisms for reading and writing files, where an LSM check is made on
open, then revalidated on read or write as a form of partial revocation.
fs/splice.c | 14 ++++++++++++++
1 files changed, 14 insertions(+), 0 deletions(-)
diff --git a/fs/splice.c b/fs/splice.c
index 12f2828..e0f68d9 100644
--- a/fs/splice.c
+++ b/fs/splice.c
@@ -28,6 +28,7 @@
#include <linux/module.h>
#include <linux/syscalls.h>
#include <linux/uio.h>
+#include <linux/security.h>
struct partial_page {
unsigned int offset;
@@ -912,6 +913,10 @@ static long do_splice_from(struct pipe_inode_info *pipe, struct file *out,
if (unlikely(ret < 0))
return ret;
+ ret = security_file_permission(out, MAY_WRITE);
+ if (unlikely(ret < 0))
+ return ret;
+
return out->f_op->splice_write(pipe, out, ppos, len, flags);
}
@@ -935,6 +940,10 @@ static long do_splice_to(struct file *in, loff_t *ppos,
if (unlikely(ret < 0))
return ret;
+ ret = security_file_permission(in, MAY_READ);
+ if (unlikely(ret < 0))
+ return ret;
+
isize = i_size_read(in->f_mapping->host);
if (unlikely(*ppos >= isize))
return 0;
@@ -1263,6 +1272,7 @@ static int get_iovec_page_array(const struct iovec __user *iov,
static long do_vmsplice(struct file *file, const struct iovec __user *iov,
unsigned long nr_segs, unsigned int flags)
{
+ long err;
struct pipe_inode_info *pipe;
struct page *pages[PIPE_BUFFERS];
struct partial_page partial[PIPE_BUFFERS];
@@ -1281,6 +1291,10 @@ static long do_vmsplice(struct file *file, const struct iovec __user *iov,
else if (unlikely(!nr_segs))
return 0;
+ err = security_file_permission(file, MAY_WRITE);
+ if (unlikely(err < 0))
+ return err;
+
spd.nr_pages = get_iovec_page_array(iov, nr_segs, pages, partial,
flags & SPLICE_F_GIFT);
if (spd.nr_pages <= 0)
--
1.5.0.6
On Tue, May 29 2007, James Morris wrote:
> Revalidate read/write permissions for splice(2) and vmslice(2), in case
> security policy has changed since the files were opened.
I guess it was inevitably going to happen :-)
Signed-off-by: Jens Axboe <[email protected]>
--
Jens Axboe
On Wed, 30 May 2007 09:15:01 +0200
Jens Axboe <[email protected]> wrote:
> On Tue, May 29 2007, James Morris wrote:
> > Revalidate read/write permissions for splice(2) and vmslice(2), in case
> > security policy has changed since the files were opened.
>
> I guess it was inevitably going to happen :-)
>
> Signed-off-by: Jens Axboe <[email protected]>
>
Probably acked-by was more appropriate.
James, is this considered needed-for-2.6.22?
> + ret = security_file_permission(out, MAY_WRITE);
> + if (unlikely(ret < 0))
> + return ret;
ot: all the unlikely()s are irksome. I wonder if there was some way of
doing this in security_file_permission() instead. eg:
static inline int security_file_permission (struct file *file, int mask)
{
int ret = security_ops->file_permission (file, mask);
if (likely(ret == 0))
return 0;
return ret;
}
On Wed, 30 May 2007, Andrew Morton wrote:
> On Wed, 30 May 2007 09:15:01 +0200
> Jens Axboe <[email protected]> wrote:
>
> > On Tue, May 29 2007, James Morris wrote:
> > > Revalidate read/write permissions for splice(2) and vmslice(2), in case
> > > security policy has changed since the files were opened.
> >
> > I guess it was inevitably going to happen :-)
> >
> > Signed-off-by: Jens Axboe <[email protected]>
> >
>
> Probably acked-by was more appropriate.
>
> James, is this considered needed-for-2.6.22?
No, it's tightening things up. We don't have full revocation in any case,
and the primary security check is performed on open.
> > + ret = security_file_permission(out, MAY_WRITE);
> > + if (unlikely(ret < 0))
> > + return ret;
>
>
> ot: all the unlikely()s are irksome. I wonder if there was some way of
> doing this in security_file_permission() instead. eg:
This is the only place where we do this -- it's to remain consistent with
the surrounding splice code, which does this for other checks. Given that
it's calling out to a typically non-trivial security module, it may be
better to remove the unlikelys here.
- James
--
James Morris
<[email protected]>