[Linux-cachefs] [PATCH] fscache: fix a kernel BUG at fs/fscache/operation.c:69!

[Lei Xue]

Hi,

To be clear, I add more details to explain this patch.  A simple example
call stack is below:

Thread #1
nfs_readpages
nfs_readpages_from_fscache
  |__nfs_readpages_from_fscache
    |__fscache_read_or_alloc_pages(...)
      | op = fscache_alloc_retrieval(...) (op->usage=1)
      | cachefiles_read_or_alloc_pages
        | cachefiles_read_backing_file
          | op->op.processor = cache_files_read_copier
          | for each netpage in list of netpages
            | fscache_get_retreival(op)   (op->usage ++)
            | add_page_wait_queue(monitor->monitor, cachefiles_read_waiter)
      | fscache_put_retrieval(op) (op->usage —)

Thread #2
cachefiles_read_waiter
 | fscache_enqueue_retrieval
    | add the monitor to op->to_do list
    | fscache_enqueue_operation
        | fscache_get_retreival(op) (op->usage ++)
        | queue_work(fscache_op_wq, &op->work) op->processor is set to
cachefiles_read_copier

Thread #3
fscache_op_work_func
 | cachefiles_read_copier
   | while (op->to_do) --> monitor {
   | copy_highpage(monitor->netfs_page, monitor->back_page);
   | put_page(monitor->back_page);
   | fscache_end_io(op, monitor->netfs_page, error); --> notify page copy
done
   | fscache_retrieval_complete(op, 1);
   | fscache_put_retrieval(op); (op->usage—)
   | kfree(monitor);
| fscache_put_retrieval(op); (op->usage --)

A potential race condition is happened on thread #2.  Assume we have two or
more pages, the second fscache_enqueue_operation may be blocked, but the
second page monitor is added in to op->to_do list.
So the thread #3 may be started before the second fscache_enqueue_operation
func, then the fscache_put_opertion may check a nil op or the op's state is
changed.

Best Regards,
-Lei

On 22 February 2018 at 15:33, <carmark.dlut at gmail.com> wrote:

> From: Lei Xue <carmark.dlut at gmail.com>
>
> There is a potential race in fscache operation enqueuing for reading and
> copying multiple pages from cachefiles to netfs.
> Under some heavy load system, it will happen very often.
>
> If this race occurs, an oops similar to the following is seen:
>
>  kernel BUG at fs/fscache/operation.c:69!
>  invalid opcode: 0000 [#1] SMP
>  …
>  #0 [ffff883fff0838d8] machine_kexec at ffffffff81051beb
>  #1 [ffff883fff083938] crash_kexec at ffffffff810f2542
>  #2 [ffff883fff083a08] oops_end at ffffffff8163e1a8
>  #3 [ffff883fff083a30] die at ffffffff8101859b
>  #4 [ffff883fff083a60] do_trap at ffffffff8163d860
>  #5 [ffff883fff083ab0] do_invalid_op at ffffffff81015204
>  #6 [ffff883fff083b60] invalid_op at ffffffff8164701e
>     [exception RIP: fscache_enqueue_operation+246]
>     RIP: ffffffffa0b793c6  RSP: ffff883fff083c18  RFLAGS: 00010046
>     RAX: 0000000000000019  RBX: ffff8832ed1a9ec0  RCX: 0000000000000006
>     RDX: 0000000000000000  RSI: 0000000000000046  RDI: 0000000000000046
>     RBP: ffff883fff083c20   R8: 0000000000000086   R9: 000000000000178f
>     R10: ffffffff816aeb00  R11: ffff883fff08392e  R12: ffff8802f0525620
>     R13: ffff88407ffc01d8  R14: 0000000000000000  R15: 0000000000000003
>     ORIG_RAX: ffffffffffffffff  CS: 0010  SS: 0000
>  #7 [ffff883fff083c10] fscache_enqueue_operation at ffffffffa0b793c6
>  #8 [ffff883fff083c28] cachefiles_read_waiter at ffffffffa0b15a48
>  #9 [ffff883fff083c48] __wake_up_common at ffffffff810af028
>
> Signed-off-by: Lei Xue <carmark.dlut at gmail.com>
> ---
>  fs/cachefiles/rdwr.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/fs/cachefiles/rdwr.c b/fs/cachefiles/rdwr.c
> index 883bc7bb12c5..9d5d13e150fb 100644
> --- a/fs/cachefiles/rdwr.c
> +++ b/fs/cachefiles/rdwr.c
> @@ -58,9 +58,9 @@ static int cachefiles_read_waiter(wait_queue_entry_t
> *wait, unsigned mode,
>
>         spin_lock(&object->work_lock);
>         list_add_tail(&monitor->op_link, &monitor->op->to_do);
> +       fscache_enqueue_retrieval(monitor->op);
>         spin_unlock(&object->work_lock);
>
> -       fscache_enqueue_retrieval(monitor->op);
>         return 0;
>  }
>
> --
> 2.14.3 (Apple Git-98)
>
>