lists.openwall.net   lists  /  announce  owl-users  owl-dev  john-users  john-dev  passwdqc-users  yescrypt  popa3d-users  /  oss-security  kernel-hardening  musl  sabotage  tlsify  passwords  /  crypt-dev  xvendor  /  Bugtraq  Full-Disclosure  linux-kernel  linux-netdev  linux-ext4  linux-hardening  linux-cve-announce  PHC 
Open Source and information security mailing list archives
 
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [day] [month] [year] [list]
Date:   Mon,  8 Jun 2020 11:56:17 -0400
From:   Xianting Tian <xianting_tian@....com>
To:     viro@...iv.linux.org.uk, linux-fsdevel@...r.kernel.org
Cc:     linux-kernel@...r.kernel.org
Subject: [PATCH] direct-io: pass correct argument to dio_complete

When submit async direct-io write operation in function
do_blockdev_direct_IO, 'struct dio' records the info of all bios,
initial value of dio->refcount is set to 1, 'dio->refcount++' is
executed in dio_bio_submit when submit one bio, 'dio->refcount--'
is executed in bio completion handler dio_bio_end_aio.

In do_blockdev_direct_IO, it also calls drop_refcount to do
'dio->refcount--', then judge if dio->refcount is 0, if yes, it
will call dio_complete to complete the dio:
    if (drop_refcount(dio) == 0) {
          retval = dio_complete(dio, retval, DIO_COMPLETE_INVALIDATE);
    } else

dio_bio_end_aio and drop_refcount will race to judge if dio->refcount
is 0:
1, if dio_bio_end_aio finds dio->refcount is 0, it will queue work if
   defer_completion is set, work handler
   dio_aio_complete_work->dio_complete will be called:
      dio_complete(dio, 0,
                    DIO_COMPLETE_ASYNC | DIO_COMPLETE_INVALIDATE);
   if defer_completion not set, it will call:
      dio_complete(dio, 0, DIO_COMPLETE_ASYNC);
   In above two cases, because DIO_COMPLETE_ASYNC is passed to
   dio_complete. So in dio_complete, it will call aio completion handler:
      dio->iocb->ki_complete(dio->iocb, ret, 0);
   As ki_complete is set to aio_complete for async io, which will fill
   an event to ring buffer, then user can use io_getevents to get this
   event.
2, if drop_refcount finds dio->refcount is 0, it will call:
      dio_complete(dio, retval, DIO_COMPLETE_INVALIDATE);
   As no DIO_COMPLETE_ASYNC is passed to dio_complete. So in dio_complete,
   ki_complete(aio_complete) will not be called. Eventually, no one fills
   the completion event to ring buffer, so user can't get the completion
   event via io_getevents.

Currently, we doesn't meet above issue with existing kernel code,
I think because do_blockdev_direct_IO is called in bio submission path,
it will be quickly completed before all aync bios completion in almost
all cases, so when drop_refcounng is executing, it finds dio->refcount is
not 0 after 'dio->refcount--'. But when the last bio completed,
dio_bio_end_aio will be called, which will find dio->refcount is 0,
then below code will be executed and the async events ring buffer getting
to be filled:
      dio_complete(dio, 0, DIO_COMPLETE_ASYNC | DIO_COMPLETE_INVALIDATE);
      or
      dio_complete(dio, 0, DIO_COMPLETE_ASYNC);

Make the code logically with this patch and cover above scenario.

Signed-off-by: Xianting Tian <xianting_tian@....com>
---
 fs/direct-io.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/fs/direct-io.c b/fs/direct-io.c
index 1543b5a..552459f 100644
--- a/fs/direct-io.c
+++ b/fs/direct-io.c
@@ -1345,7 +1345,9 @@ static inline int drop_refcount(struct dio *dio)
 		dio_await_completion(dio);
 
 	if (drop_refcount(dio) == 0) {
-		retval = dio_complete(dio, retval, DIO_COMPLETE_INVALIDATE);
+		retval = dio_complete(dio, retval, dio->is_async ?
+				DIO_COMPLETE_ASYNC | DIO_COMPLETE_INVALIDATE :
+				DIO_COMPLETE_INVALIDATE);
 	} else
 		BUG_ON(retval != -EIOCBQUEUED);
 
-- 
1.8.3.1

Powered by blists - more mailing lists