Re: [PATCH V7 09/14] powerpc/vas: Update CSB and notify process for fault CRBs

2020-03-22 Thread Haren Myneni
On Mon, 2020-03-23 at 10:06 +1000, Nicholas Piggin wrote:
> Haren Myneni's on March 18, 2020 5:27 am:
> > On Tue, 2020-03-17 at 16:28 +1100, Michael Ellerman wrote:
> >> Haren Myneni  writes:
> >> > For each fault CRB, update fault address in CRB (fault_storage_addr)
> >> > and translation error status in CSB so that user space can touch the
> >> > fault address and resend the request. If the user space passed invalid
> >> > CSB address send signal to process with SIGSEGV.
> >> >
> >> > Signed-off-by: Sukadev Bhattiprolu 
> >> > Signed-off-by: Haren Myneni 
> >> > ---
> >> >  arch/powerpc/platforms/powernv/vas-fault.c | 114 
> >> > +
> >> >  1 file changed, 114 insertions(+)
> >> >
> >> > diff --git a/arch/powerpc/platforms/powernv/vas-fault.c 
> >> > b/arch/powerpc/platforms/powernv/vas-fault.c
> >> > index 1c6d5cc..751ce48 100644
> >> > --- a/arch/powerpc/platforms/powernv/vas-fault.c
> >> > +++ b/arch/powerpc/platforms/powernv/vas-fault.c
> >> > @@ -11,6 +11,7 @@
> >> >  #include 
> >> >  #include 
> >> >  #include 
> >> > +#include 
> >> >  #include 
> >> >  #include 
> >> >  
> >> > @@ -26,6 +27,118 @@
> >> >  #define VAS_FAULT_WIN_FIFO_SIZE (4 << 20)
> >> >  
> >> >  /*
> >> > + * Update the CSB to indicate a translation error.
> >> > + *
> >> > + * If we are unable to update the CSB means copy_to_user failed due to
> >> > + * invalid csb_addr, send a signal to the process.
> >> > + *
> >> > + * Remaining settings in the CSB are based on wait_for_csb() of
> >> > + * NX-GZIP.
> >> > + */
> >> > +static void update_csb(struct vas_window *window,
> >> > +struct coprocessor_request_block *crb)
> >> > +{
> >> > +int rc;
> >> > +struct pid *pid;
> >> > +void __user *csb_addr;
> >> > +struct task_struct *tsk;
> >> > +struct kernel_siginfo info;
> >> > +struct coprocessor_status_block csb;
> >> 
> >> csb is on the stack, and later copied to user, which is a risk for
> >> creating an infoleak.
> >> 
> >> Also please use reverse Christmas tree layout for your variables.
> >> 
> >> > +
> >> > +/*
> >> > + * NX user space windows can not be opened for task->mm=NULL
> >> > + * and faults will not be generated for kernel requests.
> >> > + */
> >> > +if (!window->mm || !window->user_win)
> >> > +return;
> >> 
> >> If that's a should-never-happen condition then should it do a
> >> WARN_ON_ONCE() rather than silently returning?
> > 
> > Will add WARN_ON
> > 
> >> 
> >> > +csb_addr = (void __user *)be64_to_cpu(crb->csb_addr);
> >> > +
> >> > +csb.cc = CSB_CC_TRANSLATION;
> >> > +csb.ce = CSB_CE_TERMINATION;
> >> > +csb.cs = 0;
> >> > +csb.count = 0;
> >> > +
> >> > +/*
> >> > + * NX operates and returns in BE format as defined CRB struct.
> >> > + * So return fault_storage_addr in BE as NX pastes in FIFO and
> >> > + * expects user space to convert to CPU format.
> >> > + */
> >> > +csb.address = crb->stamp.nx.fault_storage_addr;
> >> > +csb.flags = 0;
> >> 
> >> I'm pretty sure this has initialised all the fields of csb.
> >> 
> >> But, I'd still be much happier if you zeroed the whole struct to begin
> >> with, that way we know for sure we can't leak any uninitialised bytes to
> >> userspace. It's only 16 bytes so it shouldn't add any noticeable
> >> overhead.
> > Sure, will initialize csb
> >> 
> >> > +
> >> > +pid = window->pid;
> >> > +tsk = get_pid_task(pid, PIDTYPE_PID);
> >> > +/*
> >> > + * Send window will be closed after processing all NX requests
> >> > + * and process exits after closing all windows. In multi-thread
> >> > + * applications, thread may not exists, but does not close FD
> >> > + * (means send window) upon exit. Parent thread (tgid) can use
> >> > + * and close the window later.
> >> > + * pid and mm references are taken when window is opened by
> >> > + * process (pid). So tgid is used only when child thread opens
> >> > + * a window and exits without closing it in multithread tasks.
> >> > + */
> >> > +if (!tsk) {
> >> > +pid = window->tgid;
> >> > +tsk = get_pid_task(pid, PIDTYPE_PID);
> >> > +/*
> >> > + * Parent thread will be closing window during its exit.
> >> > + * So should not get here.
> >> > + */
> >> > +if (!tsk)
> >> > +return;
> >> 
> >> Similar question on WARN_ON_ONCE()
> > Yes, we can add WARN_ON
> >> 
> >> > +}
> >> > +
> >> > +/* Return if the task is exiting. */
> >> 
> >> Why? Just because it's no use? It's racy isn't it, so it can't be for
> >> correctness?
> > Yes process is exiting and no need to update CSB. We release the
> > task->usage refcount after copy_to_user(

Re: [PATCH V7 09/14] powerpc/vas: Update CSB and notify process for fault CRBs

2020-03-22 Thread Nicholas Piggin
Haren Myneni's on March 18, 2020 5:27 am:
> On Tue, 2020-03-17 at 16:28 +1100, Michael Ellerman wrote:
>> Haren Myneni  writes:
>> > For each fault CRB, update fault address in CRB (fault_storage_addr)
>> > and translation error status in CSB so that user space can touch the
>> > fault address and resend the request. If the user space passed invalid
>> > CSB address send signal to process with SIGSEGV.
>> >
>> > Signed-off-by: Sukadev Bhattiprolu 
>> > Signed-off-by: Haren Myneni 
>> > ---
>> >  arch/powerpc/platforms/powernv/vas-fault.c | 114 
>> > +
>> >  1 file changed, 114 insertions(+)
>> >
>> > diff --git a/arch/powerpc/platforms/powernv/vas-fault.c 
>> > b/arch/powerpc/platforms/powernv/vas-fault.c
>> > index 1c6d5cc..751ce48 100644
>> > --- a/arch/powerpc/platforms/powernv/vas-fault.c
>> > +++ b/arch/powerpc/platforms/powernv/vas-fault.c
>> > @@ -11,6 +11,7 @@
>> >  #include 
>> >  #include 
>> >  #include 
>> > +#include 
>> >  #include 
>> >  #include 
>> >  
>> > @@ -26,6 +27,118 @@
>> >  #define VAS_FAULT_WIN_FIFO_SIZE   (4 << 20)
>> >  
>> >  /*
>> > + * Update the CSB to indicate a translation error.
>> > + *
>> > + * If we are unable to update the CSB means copy_to_user failed due to
>> > + * invalid csb_addr, send a signal to the process.
>> > + *
>> > + * Remaining settings in the CSB are based on wait_for_csb() of
>> > + * NX-GZIP.
>> > + */
>> > +static void update_csb(struct vas_window *window,
>> > +  struct coprocessor_request_block *crb)
>> > +{
>> > +  int rc;
>> > +  struct pid *pid;
>> > +  void __user *csb_addr;
>> > +  struct task_struct *tsk;
>> > +  struct kernel_siginfo info;
>> > +  struct coprocessor_status_block csb;
>> 
>> csb is on the stack, and later copied to user, which is a risk for
>> creating an infoleak.
>> 
>> Also please use reverse Christmas tree layout for your variables.
>> 
>> > +
>> > +  /*
>> > +   * NX user space windows can not be opened for task->mm=NULL
>> > +   * and faults will not be generated for kernel requests.
>> > +   */
>> > +  if (!window->mm || !window->user_win)
>> > +  return;
>> 
>> If that's a should-never-happen condition then should it do a
>> WARN_ON_ONCE() rather than silently returning?
> 
> Will add WARN_ON
> 
>> 
>> > +  csb_addr = (void __user *)be64_to_cpu(crb->csb_addr);
>> > +
>> > +  csb.cc = CSB_CC_TRANSLATION;
>> > +  csb.ce = CSB_CE_TERMINATION;
>> > +  csb.cs = 0;
>> > +  csb.count = 0;
>> > +
>> > +  /*
>> > +   * NX operates and returns in BE format as defined CRB struct.
>> > +   * So return fault_storage_addr in BE as NX pastes in FIFO and
>> > +   * expects user space to convert to CPU format.
>> > +   */
>> > +  csb.address = crb->stamp.nx.fault_storage_addr;
>> > +  csb.flags = 0;
>> 
>> I'm pretty sure this has initialised all the fields of csb.
>> 
>> But, I'd still be much happier if you zeroed the whole struct to begin
>> with, that way we know for sure we can't leak any uninitialised bytes to
>> userspace. It's only 16 bytes so it shouldn't add any noticeable
>> overhead.
> Sure, will initialize csb
>> 
>> > +
>> > +  pid = window->pid;
>> > +  tsk = get_pid_task(pid, PIDTYPE_PID);
>> > +  /*
>> > +   * Send window will be closed after processing all NX requests
>> > +   * and process exits after closing all windows. In multi-thread
>> > +   * applications, thread may not exists, but does not close FD
>> > +   * (means send window) upon exit. Parent thread (tgid) can use
>> > +   * and close the window later.
>> > +   * pid and mm references are taken when window is opened by
>> > +   * process (pid). So tgid is used only when child thread opens
>> > +   * a window and exits without closing it in multithread tasks.
>> > +   */
>> > +  if (!tsk) {
>> > +  pid = window->tgid;
>> > +  tsk = get_pid_task(pid, PIDTYPE_PID);
>> > +  /*
>> > +   * Parent thread will be closing window during its exit.
>> > +   * So should not get here.
>> > +   */
>> > +  if (!tsk)
>> > +  return;
>> 
>> Similar question on WARN_ON_ONCE()
> Yes, we can add WARN_ON
>> 
>> > +  }
>> > +
>> > +  /* Return if the task is exiting. */
>> 
>> Why? Just because it's no use? It's racy isn't it, so it can't be for
>> correctness?
> Yes process is exiting and no need to update CSB. We release the
> task->usage refcount after copy_to_user().
> 
>> 
>> > +  if (tsk->flags & PF_EXITING) {
>> > +  put_task_struct(tsk);
>> > +  return;
>> > +  }
>> > +
>> > +  use_mm(window->mm);
>> 
>> There's no check that csb_addr is actually pointing into userspace, but
>> copy_to_user() does it for you.
>> 
>> > +  rc = copy_to_user(csb_addr, &csb, sizeof(csb));
>> > +  /*
>> > +   * User space polls on csb.flags (first byte). So add barrier
>> > +   * then copy first byte with csb flags update.
>> > +   */
>> > +  smp_mb();
>> 
>> You only need to order the stores above vs the store below to csb.flags.
>> So you should onl

Re: [PATCH V7 09/14] powerpc/vas: Update CSB and notify process for fault CRBs

2020-03-17 Thread Haren Myneni
On Tue, 2020-03-17 at 16:28 +1100, Michael Ellerman wrote:
> Haren Myneni  writes:
> > For each fault CRB, update fault address in CRB (fault_storage_addr)
> > and translation error status in CSB so that user space can touch the
> > fault address and resend the request. If the user space passed invalid
> > CSB address send signal to process with SIGSEGV.
> >
> > Signed-off-by: Sukadev Bhattiprolu 
> > Signed-off-by: Haren Myneni 
> > ---
> >  arch/powerpc/platforms/powernv/vas-fault.c | 114 
> > +
> >  1 file changed, 114 insertions(+)
> >
> > diff --git a/arch/powerpc/platforms/powernv/vas-fault.c 
> > b/arch/powerpc/platforms/powernv/vas-fault.c
> > index 1c6d5cc..751ce48 100644
> > --- a/arch/powerpc/platforms/powernv/vas-fault.c
> > +++ b/arch/powerpc/platforms/powernv/vas-fault.c
> > @@ -11,6 +11,7 @@
> >  #include 
> >  #include 
> >  #include 
> > +#include 
> >  #include 
> >  #include 
> >  
> > @@ -26,6 +27,118 @@
> >  #define VAS_FAULT_WIN_FIFO_SIZE(4 << 20)
> >  
> >  /*
> > + * Update the CSB to indicate a translation error.
> > + *
> > + * If we are unable to update the CSB means copy_to_user failed due to
> > + * invalid csb_addr, send a signal to the process.
> > + *
> > + * Remaining settings in the CSB are based on wait_for_csb() of
> > + * NX-GZIP.
> > + */
> > +static void update_csb(struct vas_window *window,
> > +   struct coprocessor_request_block *crb)
> > +{
> > +   int rc;
> > +   struct pid *pid;
> > +   void __user *csb_addr;
> > +   struct task_struct *tsk;
> > +   struct kernel_siginfo info;
> > +   struct coprocessor_status_block csb;
> 
> csb is on the stack, and later copied to user, which is a risk for
> creating an infoleak.
> 
> Also please use reverse Christmas tree layout for your variables.
> 
> > +
> > +   /*
> > +* NX user space windows can not be opened for task->mm=NULL
> > +* and faults will not be generated for kernel requests.
> > +*/
> > +   if (!window->mm || !window->user_win)
> > +   return;
> 
> If that's a should-never-happen condition then should it do a
> WARN_ON_ONCE() rather than silently returning?

Will add WARN_ON

> 
> > +   csb_addr = (void __user *)be64_to_cpu(crb->csb_addr);
> > +
> > +   csb.cc = CSB_CC_TRANSLATION;
> > +   csb.ce = CSB_CE_TERMINATION;
> > +   csb.cs = 0;
> > +   csb.count = 0;
> > +
> > +   /*
> > +* NX operates and returns in BE format as defined CRB struct.
> > +* So return fault_storage_addr in BE as NX pastes in FIFO and
> > +* expects user space to convert to CPU format.
> > +*/
> > +   csb.address = crb->stamp.nx.fault_storage_addr;
> > +   csb.flags = 0;
> 
> I'm pretty sure this has initialised all the fields of csb.
> 
> But, I'd still be much happier if you zeroed the whole struct to begin
> with, that way we know for sure we can't leak any uninitialised bytes to
> userspace. It's only 16 bytes so it shouldn't add any noticeable
> overhead.
Sure, will initialize csb
> 
> > +
> > +   pid = window->pid;
> > +   tsk = get_pid_task(pid, PIDTYPE_PID);
> > +   /*
> > +* Send window will be closed after processing all NX requests
> > +* and process exits after closing all windows. In multi-thread
> > +* applications, thread may not exists, but does not close FD
> > +* (means send window) upon exit. Parent thread (tgid) can use
> > +* and close the window later.
> > +* pid and mm references are taken when window is opened by
> > +* process (pid). So tgid is used only when child thread opens
> > +* a window and exits without closing it in multithread tasks.
> > +*/
> > +   if (!tsk) {
> > +   pid = window->tgid;
> > +   tsk = get_pid_task(pid, PIDTYPE_PID);
> > +   /*
> > +* Parent thread will be closing window during its exit.
> > +* So should not get here.
> > +*/
> > +   if (!tsk)
> > +   return;
> 
> Similar question on WARN_ON_ONCE()
Yes, we can add WARN_ON
> 
> > +   }
> > +
> > +   /* Return if the task is exiting. */
> 
> Why? Just because it's no use? It's racy isn't it, so it can't be for
> correctness?
Yes process is exiting and no need to update CSB. We release the
task->usage refcount after copy_to_user().

> 
> > +   if (tsk->flags & PF_EXITING) {
> > +   put_task_struct(tsk);
> > +   return;
> > +   }
> > +
> > +   use_mm(window->mm);
> 
> There's no check that csb_addr is actually pointing into userspace, but
> copy_to_user() does it for you.
> 
> > +   rc = copy_to_user(csb_addr, &csb, sizeof(csb));
> > +   /*
> > +* User space polls on csb.flags (first byte). So add barrier
> > +* then copy first byte with csb flags update.
> > +*/
> > +   smp_mb();
> 
> You only need to order the stores above vs the store below to csb.flags.
> So you should only need an smp_wmb() here.
Sure, will add
if (!rc) {
csb.flags = CSB_V;
smp_mb();
rc = copy_to_user(csb_addr, &csb

Re: [PATCH V7 09/14] powerpc/vas: Update CSB and notify process for fault CRBs

2020-03-16 Thread Michael Ellerman
Haren Myneni  writes:
> For each fault CRB, update fault address in CRB (fault_storage_addr)
> and translation error status in CSB so that user space can touch the
> fault address and resend the request. If the user space passed invalid
> CSB address send signal to process with SIGSEGV.
>
> Signed-off-by: Sukadev Bhattiprolu 
> Signed-off-by: Haren Myneni 
> ---
>  arch/powerpc/platforms/powernv/vas-fault.c | 114 
> +
>  1 file changed, 114 insertions(+)
>
> diff --git a/arch/powerpc/platforms/powernv/vas-fault.c 
> b/arch/powerpc/platforms/powernv/vas-fault.c
> index 1c6d5cc..751ce48 100644
> --- a/arch/powerpc/platforms/powernv/vas-fault.c
> +++ b/arch/powerpc/platforms/powernv/vas-fault.c
> @@ -11,6 +11,7 @@
>  #include 
>  #include 
>  #include 
> +#include 
>  #include 
>  #include 
>  
> @@ -26,6 +27,118 @@
>  #define VAS_FAULT_WIN_FIFO_SIZE  (4 << 20)
>  
>  /*
> + * Update the CSB to indicate a translation error.
> + *
> + * If we are unable to update the CSB means copy_to_user failed due to
> + * invalid csb_addr, send a signal to the process.
> + *
> + * Remaining settings in the CSB are based on wait_for_csb() of
> + * NX-GZIP.
> + */
> +static void update_csb(struct vas_window *window,
> + struct coprocessor_request_block *crb)
> +{
> + int rc;
> + struct pid *pid;
> + void __user *csb_addr;
> + struct task_struct *tsk;
> + struct kernel_siginfo info;
> + struct coprocessor_status_block csb;

csb is on the stack, and later copied to user, which is a risk for
creating an infoleak.

Also please use reverse Christmas tree layout for your variables.

> +
> + /*
> +  * NX user space windows can not be opened for task->mm=NULL
> +  * and faults will not be generated for kernel requests.
> +  */
> + if (!window->mm || !window->user_win)
> + return;

If that's a should-never-happen condition then should it do a
WARN_ON_ONCE() rather than silently returning?

> + csb_addr = (void __user *)be64_to_cpu(crb->csb_addr);
> +
> + csb.cc = CSB_CC_TRANSLATION;
> + csb.ce = CSB_CE_TERMINATION;
> + csb.cs = 0;
> + csb.count = 0;
> +
> + /*
> +  * NX operates and returns in BE format as defined CRB struct.
> +  * So return fault_storage_addr in BE as NX pastes in FIFO and
> +  * expects user space to convert to CPU format.
> +  */
> + csb.address = crb->stamp.nx.fault_storage_addr;
> + csb.flags = 0;

I'm pretty sure this has initialised all the fields of csb.

But, I'd still be much happier if you zeroed the whole struct to begin
with, that way we know for sure we can't leak any uninitialised bytes to
userspace. It's only 16 bytes so it shouldn't add any noticeable
overhead.

> +
> + pid = window->pid;
> + tsk = get_pid_task(pid, PIDTYPE_PID);
> + /*
> +  * Send window will be closed after processing all NX requests
> +  * and process exits after closing all windows. In multi-thread
> +  * applications, thread may not exists, but does not close FD
> +  * (means send window) upon exit. Parent thread (tgid) can use
> +  * and close the window later.
> +  * pid and mm references are taken when window is opened by
> +  * process (pid). So tgid is used only when child thread opens
> +  * a window and exits without closing it in multithread tasks.
> +  */
> + if (!tsk) {
> + pid = window->tgid;
> + tsk = get_pid_task(pid, PIDTYPE_PID);
> + /*
> +  * Parent thread will be closing window during its exit.
> +  * So should not get here.
> +  */
> + if (!tsk)
> + return;

Similar question on WARN_ON_ONCE()

> + }
> +
> + /* Return if the task is exiting. */

Why? Just because it's no use? It's racy isn't it, so it can't be for
correctness?

> + if (tsk->flags & PF_EXITING) {
> + put_task_struct(tsk);
> + return;
> + }
> +
> + use_mm(window->mm);

There's no check that csb_addr is actually pointing into userspace, but
copy_to_user() does it for you.

> + rc = copy_to_user(csb_addr, &csb, sizeof(csb));
> + /*
> +  * User space polls on csb.flags (first byte). So add barrier
> +  * then copy first byte with csb flags update.
> +  */
> + smp_mb();

You only need to order the stores above vs the store below to csb.flags.
So you should only need an smp_wmb() here.

> + if (!rc) {
> + csb.flags = CSB_V;
> + rc = copy_to_user(csb_addr, &csb, sizeof(u8));
> + }
> + unuse_mm(window->mm);
> + put_task_struct(tsk);
> +
> + /* Success */
> + if (!rc)
> + return;
> +
> + pr_debug("Invalid CSB address 0x%p signalling pid(%d)\n",
> + csb_addr, pid_vnr(pid));
> +
> + clear_siginfo(&info);
> + info.si_signo = SIGSEGV;
> + info.si_errno = EFAULT;
> + info.si_code = SEGV_M

[PATCH V7 09/14] powerpc/vas: Update CSB and notify process for fault CRBs

2020-03-06 Thread Haren Myneni


For each fault CRB, update fault address in CRB (fault_storage_addr)
and translation error status in CSB so that user space can touch the
fault address and resend the request. If the user space passed invalid
CSB address send signal to process with SIGSEGV.

Signed-off-by: Sukadev Bhattiprolu 
Signed-off-by: Haren Myneni 
---
 arch/powerpc/platforms/powernv/vas-fault.c | 114 +
 1 file changed, 114 insertions(+)

diff --git a/arch/powerpc/platforms/powernv/vas-fault.c 
b/arch/powerpc/platforms/powernv/vas-fault.c
index 1c6d5cc..751ce48 100644
--- a/arch/powerpc/platforms/powernv/vas-fault.c
+++ b/arch/powerpc/platforms/powernv/vas-fault.c
@@ -11,6 +11,7 @@
 #include 
 #include 
 #include 
+#include 
 #include 
 #include 
 
@@ -26,6 +27,118 @@
 #define VAS_FAULT_WIN_FIFO_SIZE(4 << 20)
 
 /*
+ * Update the CSB to indicate a translation error.
+ *
+ * If we are unable to update the CSB means copy_to_user failed due to
+ * invalid csb_addr, send a signal to the process.
+ *
+ * Remaining settings in the CSB are based on wait_for_csb() of
+ * NX-GZIP.
+ */
+static void update_csb(struct vas_window *window,
+   struct coprocessor_request_block *crb)
+{
+   int rc;
+   struct pid *pid;
+   void __user *csb_addr;
+   struct task_struct *tsk;
+   struct kernel_siginfo info;
+   struct coprocessor_status_block csb;
+
+   /*
+* NX user space windows can not be opened for task->mm=NULL
+* and faults will not be generated for kernel requests.
+*/
+   if (!window->mm || !window->user_win)
+   return;
+
+   csb_addr = (void __user *)be64_to_cpu(crb->csb_addr);
+
+   csb.cc = CSB_CC_TRANSLATION;
+   csb.ce = CSB_CE_TERMINATION;
+   csb.cs = 0;
+   csb.count = 0;
+
+   /*
+* NX operates and returns in BE format as defined CRB struct.
+* So return fault_storage_addr in BE as NX pastes in FIFO and
+* expects user space to convert to CPU format.
+*/
+   csb.address = crb->stamp.nx.fault_storage_addr;
+   csb.flags = 0;
+
+   pid = window->pid;
+   tsk = get_pid_task(pid, PIDTYPE_PID);
+   /*
+* Send window will be closed after processing all NX requests
+* and process exits after closing all windows. In multi-thread
+* applications, thread may not exists, but does not close FD
+* (means send window) upon exit. Parent thread (tgid) can use
+* and close the window later.
+* pid and mm references are taken when window is opened by
+* process (pid). So tgid is used only when child thread opens
+* a window and exits without closing it in multithread tasks.
+*/
+   if (!tsk) {
+   pid = window->tgid;
+   tsk = get_pid_task(pid, PIDTYPE_PID);
+   /*
+* Parent thread will be closing window during its exit.
+* So should not get here.
+*/
+   if (!tsk)
+   return;
+   }
+
+   /* Return if the task is exiting. */
+   if (tsk->flags & PF_EXITING) {
+   put_task_struct(tsk);
+   return;
+   }
+
+   use_mm(window->mm);
+   rc = copy_to_user(csb_addr, &csb, sizeof(csb));
+   /*
+* User space polls on csb.flags (first byte). So add barrier
+* then copy first byte with csb flags update.
+*/
+   smp_mb();
+   if (!rc) {
+   csb.flags = CSB_V;
+   rc = copy_to_user(csb_addr, &csb, sizeof(u8));
+   }
+   unuse_mm(window->mm);
+   put_task_struct(tsk);
+
+   /* Success */
+   if (!rc)
+   return;
+
+   pr_debug("Invalid CSB address 0x%p signalling pid(%d)\n",
+   csb_addr, pid_vnr(pid));
+
+   clear_siginfo(&info);
+   info.si_signo = SIGSEGV;
+   info.si_errno = EFAULT;
+   info.si_code = SEGV_MAPERR;
+   info.si_addr = csb_addr;
+
+   /*
+* process will be polling on csb.flags after request is sent to
+* NX. So generally CSB update should not fail except when an
+* application does not follow the process properly. So an error
+* message will be displayed and leave it to user space whether
+* to ignore or handle this signal.
+*/
+   rcu_read_lock();
+   rc = kill_pid_info(SIGSEGV, &info, pid);
+   rcu_read_unlock();
+
+   pr_devel("%s(): pid %d kill_proc_info() rc %d\n", __func__,
+   pid_vnr(pid), rc);
+}
+
+/*
  * Process valid CRBs in fault FIFO.
  */
 irqreturn_t vas_fault_thread_fn(int irq, void *data)
@@ -111,6 +224,7 @@ irqreturn_t vas_fault_thread_fn(int irq, void *data)
return IRQ_HANDLED;
}
 
+   update_csb(window, crb);
} while (true);
 }
 
-- 
1.8.3.1