2017-07-28 15:23:01

by Ian Abbott

Subject: [PATCH] staging: comedi: comedi_fops: do not call blocking ops when !TASK_RUNNING

Comedi's read and write file operation handlers (`comedi_read()` and
`comedi_write()`) currently call `copy_to_user()` or `copy_from_user()`
whilst in the `TASK_INTERRUPTIBLE` state, which falls foul of the
`might_fault()` checks when enabled. Fix it by setting the current task
state back to `TASK_RUNNING` a bit earlier before calling these
functions.

Reported-by: Piotr Gregor <[email protected]>
Signed-off-by: Ian Abbott <[email protected]>
Cc: <[email protected]> # 4.5+
---
Note: stable kernel versions 4.4 and earlier will need a slightly more
extensive change in `comedi_write()` than provided by this patch due to
a call to `mutex_lock()` whilst in the `TASK_INTERRUPTIBLE` state.
---
drivers/staging/comedi/comedi_fops.c | 3 +++
1 file changed, 3 insertions(+)

diff --git a/drivers/staging/comedi/comedi_fops.c b/drivers/staging/comedi/comedi_fops.c
index ca11be21f64b..34ca7823255d 100644
--- a/drivers/staging/comedi/comedi_fops.c
+++ b/drivers/staging/comedi/comedi_fops.c
@@ -2396,6 +2396,7 @@ static ssize_t comedi_write(struct file *file, const char __user *buf,
continue;
}

+ set_current_state(TASK_RUNNING);
wp = async->buf_write_ptr;
n1 = min(n, async->prealloc_bufsz - wp);
n2 = n - n1;
@@ -2528,6 +2529,8 @@ static ssize_t comedi_read(struct file *file, char __user *buf, size_t nbytes,
}
continue;
}
+
+ set_current_state(TASK_RUNNING);
rp = async->buf_read_ptr;
n1 = min(n, async->prealloc_bufsz - rp);
n2 = n - n1;
--
2.13.2

2017-08-01 07:23:34

by Piotr Gregor

Subject: Re: [PATCH] staging: comedi: comedi_fops: do not call blocking ops when !TASK_RUNNING

On Fri, Jul 28, 2017 at 04:22:31PM +0100, Ian Abbott wrote:
> Comedi's read and write file operation handlers (`comedi_read()` and
> `comedi_write()`) currently call `copy_to_user()` or `copy_from_user()`
> whilst in the `TASK_INTERRUPTIBLE` state, which falls foul of the
> `might_fault()` checks when enabled. Fix it by setting the current task
> state back to `TASK_RUNNING` a bit earlier before calling these
> functions.
>
> Reported-by: Piotr Gregor <[email protected]>
> Signed-off-by: Ian Abbott <[email protected]>
> Cc: <[email protected]> # 4.5+
> ---
> Note: stable kernel versions 4.4 and earlier will need a slightly more
> extensive change in `comedi_write()` than provided by this patch due to
> a call to `mutex_lock()` whilst in the `TASK_INTERRUPTIBLE` state.
> ---
> drivers/staging/comedi/comedi_fops.c | 3 +++
> 1 file changed, 3 insertions(+)
>
> diff --git a/drivers/staging/comedi/comedi_fops.c b/drivers/staging/comedi/comedi_fops.c
> index ca11be21f64b..34ca7823255d 100644
> --- a/drivers/staging/comedi/comedi_fops.c
> +++ b/drivers/staging/comedi/comedi_fops.c
> @@ -2396,6 +2396,7 @@ static ssize_t comedi_write(struct file *file, const char __user *buf,
> continue;
> }
>
> + set_current_state(TASK_RUNNING);
> wp = async->buf_write_ptr;
> n1 = min(n, async->prealloc_bufsz - wp);
> n2 = n - n1;
> @@ -2528,6 +2529,8 @@ static ssize_t comedi_read(struct file *file, char __user *buf, size_t nbytes,
> }
> continue;
> }
> +
> + set_current_state(TASK_RUNNING);
> rp = async->buf_read_ptr;
> n1 = min(n, async->prealloc_bufsz - rp);
> n2 = n - n1;
> --
> 2.13.2
>

Hi Ian,

I will be able to test this in a couple of days.

cheers,
Piotr