Re: [PATCH 3/7] revoke: core code

2007-03-11 Thread Pekka Enberg
On Fri, 2007-03-09 at 10:15 +0200, Pekka J Enberg wrote:
> > +  again:
> > +   restart_addr = zap_page_range(vma, start_addr, end_addr - start_addr,
> > + details);
> > +
> > +   need_break = need_resched() || need_lockbreak(details->i_mmap_lock);
> > +   if (need_break)
> > +   goto out_need_break;
> > +
> > +   if (restart_addr < end_addr) {
> > +   start_addr = restart_addr;
> > +   goto again;
> > +   }
> > +   return 0;
> > +
> > +  out_need_break:
> > +   spin_unlock(details->i_mmap_lock);
> > +   cond_resched();
> > +   spin_lock(details->i_mmap_lock);
> > +   return -EINTR;

On Fri, 2007-03-09 at 13:30 +0100, Peter Zijlstra wrote:
> I'm not sure this scheme works, given a sufficiently loaded machine,
> this might never complete.

Hmm, so what's the alternative? It's better to fail revoke than lock up
the box.

On Fri, 2007-03-09 at 13:30 +0100, Peter Zijlstra wrote:
> I'm never sure of operator precedence and prefer:
> 
>  (vma->vm_flags & VM_SHARED) && ...
> 
> which leaves no room for error.

Thanks, fixed.

-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH 3/7] revoke: core code

2007-03-11 Thread Pekka Enberg
On Fri, 2007-03-09 at 10:15 +0200, Pekka J Enberg wrote:
  +  again:
  +   restart_addr = zap_page_range(vma, start_addr, end_addr - start_addr,
  + details);
  +
  +   need_break = need_resched() || need_lockbreak(details-i_mmap_lock);
  +   if (need_break)
  +   goto out_need_break;
  +
  +   if (restart_addr  end_addr) {
  +   start_addr = restart_addr;
  +   goto again;
  +   }
  +   return 0;
  +
  +  out_need_break:
  +   spin_unlock(details-i_mmap_lock);
  +   cond_resched();
  +   spin_lock(details-i_mmap_lock);
  +   return -EINTR;

On Fri, 2007-03-09 at 13:30 +0100, Peter Zijlstra wrote:
 I'm not sure this scheme works, given a sufficiently loaded machine,
 this might never complete.

Hmm, so what's the alternative? It's better to fail revoke than lock up
the box.

On Fri, 2007-03-09 at 13:30 +0100, Peter Zijlstra wrote:
 I'm never sure of operator precedence and prefer:
 
  (vma-vm_flags  VM_SHARED)  ...
 
 which leaves no room for error.

Thanks, fixed.

-
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH 3/7] revoke: core code

2007-03-09 Thread Peter Zijlstra
On Fri, 2007-03-09 at 10:15 +0200, Pekka J Enberg wrote:

> +static int revoke_vma(struct vm_area_struct *vma, struct zap_details 
> *details)
> +{
> + unsigned long restart_addr, start_addr, end_addr;
> + int need_break;
> +
> + start_addr = vma->vm_start;
> + end_addr = vma->vm_end;
> +
> + /*
> +  * Not holding ->mmap_sem here.
> +  */
> + vma->vm_flags |= VM_REVOKED;
> + smp_mb();

Hmm, i_mmap_lock pins the vma and excludes modifications, but doesn't
exclude concurrent faults.

I guess its save.

> +  again:
> + restart_addr = zap_page_range(vma, start_addr, end_addr - start_addr,
> +   details);
> +
> + need_break = need_resched() || need_lockbreak(details->i_mmap_lock);
> + if (need_break)
> + goto out_need_break;
> +
> + if (restart_addr < end_addr) {
> + start_addr = restart_addr;
> + goto again;
> + }
> + return 0;
> +
> +  out_need_break:
> + spin_unlock(details->i_mmap_lock);
> + cond_resched();
> + spin_lock(details->i_mmap_lock);
> + return -EINTR;

I'm not sure this scheme works, given a sufficiently loaded machine,
this might never complete.

> +}
> +
> +static int revoke_mapping(struct address_space *mapping, struct file 
> *to_exclude)
> +{
> + struct vm_area_struct *vma;
> + struct prio_tree_iter iter;
> + struct zap_details details;
> + int err = 0;
> +
> + details.i_mmap_lock = >i_mmap_lock;
> +
> + spin_lock(>i_mmap_lock);
> + vma_prio_tree_foreach(vma, , >i_mmap, 0, ULONG_MAX) {
> + if (vma->vm_flags & VM_SHARED && vma->vm_file != to_exclude) {

I'm never sure of operator precedence and prefer:

 (vma->vm_flags & VM_SHARED) && ...

which leaves no room for error.

> + err = revoke_vma(vma, );
> + if (err)
> + goto out;
> + }
> + }
> +
> + list_for_each_entry(vma, >i_mmap_nonlinear, 
> shared.vm_set.list) {
> + if (vma->vm_flags & VM_SHARED && vma->vm_file != to_exclude) {

Idem.

> + err = revoke_vma(vma, );
> + if (err)
> + goto out;
> + }
> + }
> +  out:
> + spin_unlock(>i_mmap_lock);
> + return err;
> +}


-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH 3/7] revoke: core code

2007-03-09 Thread Pekka J Enberg
On Fri, 9 Mar 2007 10:15:15 +0200 (EET)
Pekka J Enberg <[EMAIL PROTECTED]> wrote:
> > +static ssize_t revoked_file_aio_read(struct kiocb *iocb,
> > +const struct iovec *iov,
> > +unsigned long nr_segs, loff_t pos)
> > +{
> > +   return -EBADF;
> > +}
 
On Fri, 9 Mar 2007, Alan Cox wrote:
> Do we need both -EBADF and -EXNIO versions. It is hard to tell from
> existing OS's as they don't support revoke of files just special files ?

No, we don't. We should always do EBADF except for close(2) which is zero 
always and make read(2) zero for special files.

On Fri, 9 Mar 2007 10:15:15 +0200 (EET)
Pekka J Enberg <[EMAIL PROTECTED]> wrote:
> > +static ssize_t revoked_special_file_read(struct file *filp, char __user * 
> > buf,
> > +size_t size, loff_t * ppos)
> > +{
> > +   return -ENXIO;
> > +}

On Fri, 9 Mar 2007, Alan Cox wrote:
> Bezerkly Unix returns 0 for the special file read case

Aah, I'll fix that up. Thanks.

Pekka
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH 3/7] revoke: core code

2007-03-09 Thread Alan Cox
On Fri, 9 Mar 2007 10:15:15 +0200 (EET)
Pekka J Enberg <[EMAIL PROTECTED]> wrote:

> From: Pekka Enberg <[EMAIL PROTECTED]>
> 
> The revokeat(2) and frevoke(2) system calls invalidate open file
> descriptors and shared mappings of an inode. After an successful
> revocation, operations on file descriptors fail with the EBADF or
> ENXIO error code for regular and device files,
> respectively. Attempting to read from or write to a revoked mapping
> causes SIGBUS.

Acked-by: Alan Cox <[EMAIL PROTECTED]>


> +static ssize_t revoked_file_aio_read(struct kiocb *iocb,
> +  const struct iovec *iov,
> +  unsigned long nr_segs, loff_t pos)
> +{
> + return -EBADF;
> +}

Do we need both -EBADF and -EXNIO versions. It is hard to tell from
existing OS's as they don't support revoke of files just special files ?

> +static ssize_t revoked_special_file_read(struct file *filp, char __user * 
> buf,
> +  size_t size, loff_t * ppos)
> +{
> + return -ENXIO;
> +}

Bezerkly Unix returns 0 for the special file read case


-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH 3/7] revoke: core code

2007-03-09 Thread Alan Cox
On Fri, 9 Mar 2007 10:15:15 +0200 (EET)
Pekka J Enberg [EMAIL PROTECTED] wrote:

 From: Pekka Enberg [EMAIL PROTECTED]
 
 The revokeat(2) and frevoke(2) system calls invalidate open file
 descriptors and shared mappings of an inode. After an successful
 revocation, operations on file descriptors fail with the EBADF or
 ENXIO error code for regular and device files,
 respectively. Attempting to read from or write to a revoked mapping
 causes SIGBUS.

Acked-by: Alan Cox [EMAIL PROTECTED]


 +static ssize_t revoked_file_aio_read(struct kiocb *iocb,
 +  const struct iovec *iov,
 +  unsigned long nr_segs, loff_t pos)
 +{
 + return -EBADF;
 +}

Do we need both -EBADF and -EXNIO versions. It is hard to tell from
existing OS's as they don't support revoke of files just special files ?

 +static ssize_t revoked_special_file_read(struct file *filp, char __user * 
 buf,
 +  size_t size, loff_t * ppos)
 +{
 + return -ENXIO;
 +}

Bezerkly Unix returns 0 for the special file read case


-
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH 3/7] revoke: core code

2007-03-09 Thread Pekka J Enberg
On Fri, 9 Mar 2007 10:15:15 +0200 (EET)
Pekka J Enberg [EMAIL PROTECTED] wrote:
  +static ssize_t revoked_file_aio_read(struct kiocb *iocb,
  +const struct iovec *iov,
  +unsigned long nr_segs, loff_t pos)
  +{
  +   return -EBADF;
  +}
 
On Fri, 9 Mar 2007, Alan Cox wrote:
 Do we need both -EBADF and -EXNIO versions. It is hard to tell from
 existing OS's as they don't support revoke of files just special files ?

No, we don't. We should always do EBADF except for close(2) which is zero 
always and make read(2) zero for special files.

On Fri, 9 Mar 2007 10:15:15 +0200 (EET)
Pekka J Enberg [EMAIL PROTECTED] wrote:
  +static ssize_t revoked_special_file_read(struct file *filp, char __user * 
  buf,
  +size_t size, loff_t * ppos)
  +{
  +   return -ENXIO;
  +}

On Fri, 9 Mar 2007, Alan Cox wrote:
 Bezerkly Unix returns 0 for the special file read case

Aah, I'll fix that up. Thanks.

Pekka
-
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH 3/7] revoke: core code

2007-03-09 Thread Peter Zijlstra
On Fri, 2007-03-09 at 10:15 +0200, Pekka J Enberg wrote:

 +static int revoke_vma(struct vm_area_struct *vma, struct zap_details 
 *details)
 +{
 + unsigned long restart_addr, start_addr, end_addr;
 + int need_break;
 +
 + start_addr = vma-vm_start;
 + end_addr = vma-vm_end;
 +
 + /*
 +  * Not holding -mmap_sem here.
 +  */
 + vma-vm_flags |= VM_REVOKED;
 + smp_mb();

Hmm, i_mmap_lock pins the vma and excludes modifications, but doesn't
exclude concurrent faults.

I guess its save.

 +  again:
 + restart_addr = zap_page_range(vma, start_addr, end_addr - start_addr,
 +   details);
 +
 + need_break = need_resched() || need_lockbreak(details-i_mmap_lock);
 + if (need_break)
 + goto out_need_break;
 +
 + if (restart_addr  end_addr) {
 + start_addr = restart_addr;
 + goto again;
 + }
 + return 0;
 +
 +  out_need_break:
 + spin_unlock(details-i_mmap_lock);
 + cond_resched();
 + spin_lock(details-i_mmap_lock);
 + return -EINTR;

I'm not sure this scheme works, given a sufficiently loaded machine,
this might never complete.

 +}
 +
 +static int revoke_mapping(struct address_space *mapping, struct file 
 *to_exclude)
 +{
 + struct vm_area_struct *vma;
 + struct prio_tree_iter iter;
 + struct zap_details details;
 + int err = 0;
 +
 + details.i_mmap_lock = mapping-i_mmap_lock;
 +
 + spin_lock(mapping-i_mmap_lock);
 + vma_prio_tree_foreach(vma, iter, mapping-i_mmap, 0, ULONG_MAX) {
 + if (vma-vm_flags  VM_SHARED  vma-vm_file != to_exclude) {

I'm never sure of operator precedence and prefer:

 (vma-vm_flags  VM_SHARED)  ...

which leaves no room for error.

 + err = revoke_vma(vma, details);
 + if (err)
 + goto out;
 + }
 + }
 +
 + list_for_each_entry(vma, mapping-i_mmap_nonlinear, 
 shared.vm_set.list) {
 + if (vma-vm_flags  VM_SHARED  vma-vm_file != to_exclude) {

Idem.

 + err = revoke_vma(vma, details);
 + if (err)
 + goto out;
 + }
 + }
 +  out:
 + spin_unlock(mapping-i_mmap_lock);
 + return err;
 +}


-
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/