Re: [PATCH 0/15 v2] loop: Fix oops and possible deadlocks

On Wed, Oct 10, 2018 at 02:28:09PM +0200, Jan Kara wrote: > On Wed 10-10-18 13:42:27, Johannes Thumshirn wrote: > > On Wed, Oct 10, 2018 at 07:19:00PM +0900, Tetsuo Handa wrote: > > > On 2018/10/10 19:04, Jan Kara wrote: > > > > Hi, > > > > > > > > this patch series fixes oops and possible

Re: [PATCH 0/15 v2] loop: Fix oops and possible deadlocks

On Wed 10-10-18 13:42:27, Johannes Thumshirn wrote: > On Wed, Oct 10, 2018 at 07:19:00PM +0900, Tetsuo Handa wrote: > > On 2018/10/10 19:04, Jan Kara wrote: > > > Hi, > > > > > > this patch series fixes oops and possible deadlocks as reported by syzbot > > > [1] > > > [2]. The second patch in

Re: [PATCH 0/15 v2] loop: Fix oops and possible deadlocks

On Wed, Oct 10, 2018 at 07:19:00PM +0900, Tetsuo Handa wrote: > On 2018/10/10 19:04, Jan Kara wrote: > > Hi, > > > > this patch series fixes oops and possible deadlocks as reported by syzbot > > [1] > > [2]. The second patch in the series (from Tetsuo) fixes the oops, the > > remaining > >

Re: [PATCH 0/15 v2] loop: Fix oops and possible deadlocks

On 2018/10/10 19:04, Jan Kara wrote: > Hi, > > this patch series fixes oops and possible deadlocks as reported by syzbot [1] > [2]. The second patch in the series (from Tetsuo) fixes the oops, the > remaining > patches are cleaning up the locking in the loop driver so that we can in the > end

[PATCH 06/15] loop: Split setting of lo_state from loop_clr_fd

Move setting of lo_state to Lo_rundown out into the callers. That will allow us to unlock loop_ctl_mutex while the loop device is protected from other changes by its special state. Signed-off-by: Jan Kara --- drivers/block/loop.c | 52 +++- 1 file

[PATCH 15/15] loop: Avoid circular locking dependency between loop_ctl_mutex and bd_mutex

Code in loop_change_fd() drops reference to the old file (and also the new file in a failure case) under loop_ctl_mutex. Similarly to a situation in loop_set_fd() this can create a circular locking dependency if this was the last reference holding the file open. Delay dropping of the file

[PATCH 07/15] loop: Push loop_ctl_mutex down into loop_clr_fd()

loop_clr_fd() has a weird locking convention that is expects loop_ctl_mutex held, releases it on success and keeps it on failure. Untangle the mess by moving locking of loop_ctl_mutex into loop_clr_fd(). Signed-off-by: Jan Kara --- drivers/block/loop.c | 49

[PATCH 01/15] block/loop: Don't grab "struct file" for vfs_getattr() operation.

From: Tetsuo Handa vfs_getattr() needs "struct path" rather than "struct file". Let's use path_get()/path_put() rather than get_file()/fput(). Signed-off-by: Tetsuo Handa Reviewed-by: Jan Kara Signed-off-by: Jan Kara --- drivers/block/loop.c | 10 +- 1 file changed, 5 insertions(+),

[PATCH 11/15] loop: Push loop_ctl_mutex down to loop_change_fd()

Push loop_ctl_mutex down to loop_change_fd(). We will need this to be able to call loop_reread_partitions() without loop_ctl_mutex. Signed-off-by: Jan Kara --- drivers/block/loop.c | 22 +++--- 1 file changed, 11 insertions(+), 11 deletions(-) diff --git a/drivers/block/loop.c

[PATCH 03/15] loop: Fold __loop_release into loop_release

__loop_release() has a single call site. Fold it there. This is currently not a huge win but it will make following replacement of loop_index_mutex more obvious. Signed-off-by: Jan Kara --- drivers/block/loop.c | 16 +++- 1 file changed, 7 insertions(+), 9 deletions(-) diff --git

[PATCH 05/15] loop: Push lo_ctl_mutex down into individual ioctls

Push acquisition of lo_ctl_mutex down into individual ioctl handling branches. This is a preparatory step for pushing the lock down into individual ioctl handling functions so that they can release the lock as they need it. We also factor out some simple ioctl handlers that will not need any

[PATCH 09/15] loop: Push loop_ctl_mutex down to loop_set_status()

Push loop_ctl_mutex down to loop_set_status(). We will need this to be able to call loop_reread_partitions() without loop_ctl_mutex. Signed-off-by: Jan Kara --- drivers/block/loop.c | 51 +-- 1 file changed, 25 insertions(+), 26 deletions(-) diff

[PATCH 14/15] loop: Fix deadlock when calling blkdev_reread_part()

Calling blkdev_reread_part() under loop_ctl_mutex causes lockdep to complain about circular lock dependency between bdev->bd_mutex and lo->lo_ctl_mutex. The problem is that on loop device open or close lo_open() and lo_release() get called with bdev->bd_mutex held and they need to acquire

[PATCH 08/15] loop: Push loop_ctl_mutex down to loop_get_status()

Push loop_ctl_mutex down to loop_get_status() to avoid the unusual convention that the function gets called with loop_ctl_mutex held and releases it. Signed-off-by: Jan Kara --- drivers/block/loop.c | 37 ++--- 1 file changed, 10 insertions(+), 27 deletions(-)

[PATCH 12/15] loop: Move special partition reread handling in loop_clr_fd()

The call of __blkdev_reread_part() from loop_reread_partition() happens only when we need to invalidate partitions from loop_release(). Thus move a detection for this into loop_clr_fd() and simplify loop_reread_partition(). This makes loop_reread_partition() safe to use without loop_ctl_mutex

[PATCH 04/15] loop: Get rid of loop_index_mutex

Now that loop_ctl_mutex is global, just get rid of loop_index_mutex as there is no good reason to keep these two separate and it just complicates the locking. Signed-off-by: Jan Kara --- drivers/block/loop.c | 41 - 1 file changed, 20 insertions(+), 21

[PATCH 02/15] block/loop: Use global lock for ioctl() operation.

From: Tetsuo Handa syzbot is reporting NULL pointer dereference [1] which is caused by race condition between ioctl(loop_fd, LOOP_CLR_FD, 0) versus ioctl(other_loop_fd, LOOP_SET_FD, loop_fd) due to traversing other loop devices at loop_validate_file() without holding corresponding

[PATCH 10/15] loop: Push loop_ctl_mutex down to loop_set_fd()

Push lo_ctl_mutex down to loop_set_fd(). We will need this to be able to call loop_reread_partitions() without lo_ctl_mutex. Signed-off-by: Jan Kara --- drivers/block/loop.c | 26 ++ 1 file changed, 14 insertions(+), 12 deletions(-) diff --git a/drivers/block/loop.c

[PATCH 0/15 v2] loop: Fix oops and possible deadlocks

Hi, this patch series fixes oops and possible deadlocks as reported by syzbot [1] [2]. The second patch in the series (from Tetsuo) fixes the oops, the remaining patches are cleaning up the locking in the loop driver so that we can in the end reasonably easily switch to rereading partitions