space calls io_destroy() while another process is polling
for events on the same kioctx? It seems we'd be reaping events from two
processes in parallel in that case which will result in various
"interesting" effects like ctx->poll_completing list corruption...
Honza
--
Jan Kara
SUSE Labs, CR
On Thu 08-11-18 16:28:11, Theodore Y. Ts'o wrote:
> On Thu, Nov 08, 2018 at 02:01:00PM +0100, 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) f
On Thu 08-11-18 06:21:21, Jens Axboe wrote:
> On 11/8/18 6:01 AM, 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
> >
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
roperly fixed, let's stop fooling lockdep.
Signed-off-by: Jan Kara
---
drivers/block/loop.c | 12 ++--
1 file changed, 6 insertions(+), 6 deletions(-)
diff --git a/drivers/block/loop.c b/drivers/block/loop.c
index 112afc9bc604..bf6bc35aaf88 100644
--- a/drivers/block/loop.c
+++ b/drivers
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
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
fix deadlock possibility.
[1] https://syzkaller.appspot.com/bug?id=bf154052f0eea4bc7712499e4569505907d1588
Reported-by: syzbot
Signed-off-by: Jan Kara
---
drivers/block/loop.c | 19 ++-
1 file changed, 14 insertions(+), 5 deletions(-)
diff --git a/drivers/block/loop.c b/drivers/block/
kdep warning and the possible
deadlock.
[1] https://syzkaller.appspot.com/bug?id=bf154052f0eea4bc7712499e4569505907d1588
Reported-by: syzbot
Signed-off-by: Jan Kara
---
drivers/block/loop.c | 28
1 file changed, 16 insertions(+), 12 deletions(-)
diff --git a/d
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
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
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 c
reference until we have released loop_ctl_mutex.
Reported-by: Tetsuo Handa
Signed-off-by: Jan Kara
---
drivers/block/loop.c | 26 +++---
1 file changed, 15 insertions(+), 11 deletions(-)
diff --git a/drivers/block/loop.c b/drivers/block/loop.c
index b3f981ac8ef1..112afc9bc604 100644
special handling to reduce unnecessary code duplication.
Signed-off-by: Jan Kara
---
drivers/block/loop.c | 88 +---
1 file changed, 63 insertions(+), 25 deletions(-)
diff --git a/drivers/block/loop.c b/drivers/block/loop.c
index dcdc96f4d2d4
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
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
suo Handa
Reported-by: syzbot
Reviewed-by: Jan Kara
Signed-off-by: Jan Kara
---
drivers/block/loop.c | 58 ++--
drivers/block/loop.h | 1 -
2 files changed, 29 insertions(+), 30 deletions(-)
diff --git a/drivers/block/loop.c b/drivers/block/loo
because we use only lo->lo_number and lo->lo_file_name in case of error
for reporting purposes (thus possibly reporting outdate information is
not a big deal) and we are safe from 'lo' going away under us by
elevated lo->lo_refcnt.
Signed-off-by: Jan Kara
---
drivers/block/lo
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 b
__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
On Tue 23-10-18 11:57:50, Omar Sandoval wrote:
> On Tue, Oct 23, 2018 at 12:05:12PM +0200, Jan Kara wrote:
> > On Mon 22-10-18 15:52:55, Omar Sandoval wrote:
> > > On Thu, Oct 18, 2018 at 12:31:46PM +0200, Jan Kara wrote:
> > > > Add test for setting partscan fla
On Mon 22-10-18 15:52:55, Omar Sandoval wrote:
> On Thu, Oct 18, 2018 at 12:31:46PM +0200, Jan Kara wrote:
> > Add test for setting partscan flag.
> >
> > Signed-off-by: Jan Kara
>
> Sorry I didn't notice this earlier, but loop/001 already does a
> partition
Add regression test for patch "block/loop: Use global lock for ioctl()
operation." where we can oops while traversing list of loop devices
backing newly created device.
Signed-off-by: Jan Kara
---
src/Makefile | 3 ++-
src/loop_change_fd.c | 48 ++
Hello,
these two patches create two new tests for blktests as regression tests
for my recently posted loopback device fixes. More details in individual
patches.
Honza
Add test for setting partscan flag.
Signed-off-by: Jan Kara
---
src/Makefile | 3 ++-
src/loop_set_status_partscan.c | 45 ++
tests/loop/006 | 33 +++
tests/loop/006.out | 2 ++
4
On Wed 17-10-18 10:29:22, Jens Axboe wrote:
> On 10/17/18 4:05 AM, Jan Kara wrote:
> > On Tue 16-10-18 11:35:59, Jens Axboe wrote:
> >> On 10/15/18 1:44 PM, Paolo Valente wrote:
> >>> Here are some old results with a very simple configuration:
> >>> http:/
they
either don't run udev or they feel not all their teams building new
products have enough expertise to come up with a proper set of rules...
Honza
--
Jan Kara
SUSE Labs, CR
On Tue 16-10-18 11:16:22, Omar Sandoval wrote:
> On Tue, Oct 16, 2018 at 01:36:54PM +0200, Jan Kara wrote:
> > On Wed 10-10-18 14:28:09, 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
On Wed 10-10-18 14:28:09, 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 patc
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
&
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
reference until we have released loop_ctl_mutex.
Reported-by: Tetsuo Handa
Signed-off-by: Jan Kara
---
drivers/block/loop.c | 26 +++---
1 file changed, 15 insertions(+), 11 deletions(-)
diff --git a/drivers/block/loop.c b/drivers/block/loop.c
index d6b3fac25040..a517247a32fa 100644
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
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 c
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
__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
special handling to reduce unnecessary code duplication.
Signed-off-by: Jan Kara
---
drivers/block/loop.c | 88 +---
1 file changed, 63 insertions(+), 25 deletions(-)
diff --git a/drivers/block/loop.c b/drivers/block/loop.c
index 3fa5e63944a4
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
kdep warning and the possible
deadlock.
[1] https://syzkaller.appspot.com/bug?id=bf154052f0eea4bc7712499e4569505907d1588
Reported-by: syzbot
Signed-off-by: Jan Kara
---
drivers/block/loop.c | 28
1 file changed, 16 insertions(+), 12 deletions(-)
diff --git a/d
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
because we use only lo->lo_number and lo->lo_file_name in case of error
for reporting purposes (thus possibly reporting outdate information is
not a big deal) and we are safe from 'lo' going away under us by
elevated lo->lo_refcnt.
Signed-off-by: Jan Kara
---
drivers/block/lo
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
suo Handa
Reported-by: syzbot
Reviewed-by: Jan Kara
Signed-off-by: Jan Kara
---
drivers/block/loop.c | 58 ++--
drivers/block/loop.h | 1 -
2 files changed, 29 insertions(+), 30 deletions(-)
diff --git a/drivers/block/loop.c b/drivers/block/loo
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 b
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
.
This is deliberate so that we get rid of the weird "__loop_clr_fd()
releases mutex it did not acquire". This is not performance critical path
by any means so better keep the locking simple.
Honza
--
Jan Kara
SUSE Labs, CR
On Thu 27-09-18 20:35:27, Tetsuo Handa wrote:
> On 2018/09/27 20:27, Jan Kara wrote:
> > Hi,
> >
> > On Wed 26-09-18 00:26:49, Tetsuo Handa wrote:
> >> syzbot is reporting circular locking dependency between bdev->bd_mutex
> >> and lo-&g
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
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 | 38 ++
1 file changed, 18 insertions(+), 20
fix deadlock possibility.
[1] https://syzkaller.appspot.com/bug?id=bf154052f0eea4bc7712499e4569505907d1588
Reported-by: syzbot
Signed-off-by: Jan Kara
---
drivers/block/loop.c | 19 ++-
1 file changed, 14 insertions(+), 5 deletions(-)
diff --git a/drivers/block/loop.c b/drivers/block/
__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
kdep warning and the possible
deadlock.
[1] https://syzkaller.appspot.com/bug?id=bf154052f0eea4bc7712499e4569505907d1588
Reported-by: syzbot
Signed-off-by: Jan Kara
---
drivers/block/loop.c | 28
1 file changed, 16 insertions(+), 12 deletions(-)
diff --git a/d
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
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 b
because we use only lo->lo_number and lo->lo_file_name in case of error
for reporting purposes (thus possibly reporting outdate information is
not a big deal) and we are safe from 'lo' going away under us by
elevated lo->lo_refcnt.
Signed-off-by: Jan Kara
---
drivers/block/lo
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
special handling to reduce unnecessary code duplication.
Signed-off-by: Jan Kara
---
drivers/block/loop.c | 88 +---
1 file changed, 63 insertions(+), 25 deletions(-)
diff --git a/drivers/block/loop.c b/drivers/block/loop.c
index e35707fb8318
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
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
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
suo Handa
Reported-by: syzbot
Reviewed-by: Jan Kara
Signed-off-by: Jan Kara
---
drivers/block/loop.c | 58 ++--
drivers/block/loop.h | 1 -
2 files changed, 29 insertions(+), 30 deletions(-)
diff --git a/drivers/block/loop.c b/drivers/block/loo
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 c
nt ret = -ENOSYS;
> + int ret = lock_loop_killable();
>
> - mutex_lock(_index_mutex);
> + if (ret)
> + return ret;
> + ret = -ENOSYS;
> switch (cmd) {
> case LOOP_CTL_ADD:
> ret = loop_lookup(, parm);
> @@ -2019,21 +2040,15 @@ static long loop_control_ioctl(struct file *file,
> unsigned int cmd,
> ret = loop_lookup(, parm);
> if (ret < 0)
> break;
> - ret = mutex_lock_killable(_ctl_mutex);
> - if (ret)
> - break;
> if (lo->lo_state != Lo_unbound) {
> ret = -EBUSY;
> - mutex_unlock(_ctl_mutex);
> break;
> }
> if (atomic_read(>lo_refcnt) > 0) {
> ret = -EBUSY;
> - mutex_unlock(_ctl_mutex);
> break;
> }
> lo->lo_disk->private_data = NULL;
> - mutex_unlock(_ctl_mutex);
> idr_remove(_index_idr, lo->lo_number);
> loop_remove(lo);
> break;
> @@ -2043,7 +2058,7 @@ static long loop_control_ioctl(struct file *file,
> unsigned int cmd,
> break;
> ret = loop_add(, -1);
> }
> - mutex_unlock(_index_mutex);
> + unlock_loop();
>
> return ret;
> }
> @@ -2127,10 +2142,10 @@ static int __init loop_init(void)
> THIS_MODULE, loop_probe, NULL, NULL);
>
> /* pre-create number of devices given by config or max_loop */
> - mutex_lock(_index_mutex);
> + lock_loop();
> for (i = 0; i < nr; i++)
> loop_add(, i);
> - mutex_unlock(_index_mutex);
> + unlock_loop();
>
> printk(KERN_INFO "loop: module loaded\n");
> return 0;
> --
> 1.8.3.1
>
--
Jan Kara
SUSE Labs, CR
partitions() and reorganizing the code there so
that loop_reread_partitions() is called as late as possible so that it is
clear that dropping the mutex is fine (and usually we don't even have to
reacquire it). Plus your patch does not seem to take care of the possible
races of loop_clr_fd() with LOOP_CTL_REMOVE? See my other mail for
details...
Honza
--
Jan Kara
SUSE Labs, CR
oks good to me. Feel free
to add:
Reviewed-by: Jan Kara
Honza
> ---
> drivers/block/loop.c | 10 +-
> 1 file changed, 5 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/block/loop.c b/drivers/block/loop.c
On Wed 22-08-18 18:50:53, Ming Lei wrote:
> On Wed, Aug 22, 2018 at 12:33:05PM +0200, Jan Kara wrote:
> > On Wed 22-08-18 10:02:49, Martin Wilck wrote:
> > > On Mon, 2018-07-30 at 20:37 +0800, Ming Lei wrote:
> > > > On Wed, Jul 25, 2018 at 11:15:
ADs as well?
>
> I haven't encountered this situation in my tests, and I'm unsure how to
> provoke it - run a direct IO test under high memory pressure?
Currently, iov_iter_get_pages() is always guaranteed to get at least one
page as that is current guarantee of get_user_pages() (unless we hit
EFAULT obviously). So bio_iov_iter_get_pages() as is now is guaranteed to
exhaust 'iter' or fill 'bio'. But in the future, the guarantee that
get_user_pages() will always pin at least one page may go away. But we'd
have to audit all users at that time anyway.
Honza
--
Jan Kara
SUSE Labs, CR
On Thu 19-07-18 20:20:51, Ming Lei wrote:
> On Thu, Jul 19, 2018 at 01:56:16PM +0200, Jan Kara wrote:
> > On Thu 19-07-18 19:04:46, Ming Lei wrote:
> > > On Thu, Jul 19, 2018 at 11:39:18AM +0200, Martin Wilck wrote:
> > > > bio_iov_iter_get_pages() returns onl
On Thu 19-07-18 14:23:53, Martin Wilck wrote:
> On Thu, 2018-07-19 at 12:45 +0200, Jan Kara wrote:
> > Secondly, I don't think it is good to discard error from
> > bio_iov_iter_get_pages() here and just submit partial IO. It will
> > again
> > lead to part of IO be
rates. Martin optimized this to fill the bio
completely (as we know we have enough bvecs) before submitting which has
chances to perform better. I'm fine with either approach, just we have to
decide which way to go.
Honza
--
Jan Kara
SUSE Labs, CR
t;
>
> You might put the 'vecs' leak fix into another patch, and resue the
> current code block for that.
>
> Looks all users of bio_iov_iter_get_pages() need this kind of fix, so
> what do you think about the following way?
No. AFAICT all the other users of bio_iov_iter_get_pages() are perfectly
fine with it returning less pages and they loop appropriately.
Honza
--
Jan Kara
SUSE Labs, CR
one as direct and part attempted to be done as
buffered. Also the "slow" direct IO path in __blkdev_direct_IO() behaves
differently - it aborts and returns error if bio_iov_iter_get_pages() ever
returned error. IMO we should do the same here.
Honza
--
Jan Kara
SUSE Labs, CR
ork as expected?
Well, there has never been a promise that it will grab *all* pages in the
iter AFAIK. Practically, I think that it was just too hairy to implement in
the macro magic that iter processing is... Al might know more (added to
CC).
Honza
--
Jan Kara
SUSE Labs, CR
s shifted by the value of bio->bi_vcnt at function
> invocation, the correct index is (nr_pages - 1).
>
> V2: improved readability following suggestions from Ming Lei.
>
> Fixes: 2cefe4dbaadf ("block: add bio_iov_iter_get_pages()")
> Signed-off-by: Martin Wilck
Loo
On Wed 18-07-18 13:40:07, Jan Kara wrote:
> On Wed 18-07-18 11:20:15, Johannes Thumshirn wrote:
> > On Wed, Jul 18, 2018 at 03:54:46PM +0800, Ming Lei wrote:
> > > Please go ahead and take care of it since you have the test cases.
> >
> > Speaking of which, do we al
le
the second ends up doing read-modify-write cycle through page cache, the
first write could end up being lost. I'll try whether something like this
is able to see the corruption...
Honza
--
Jan Kara
SUSE Labs, CR
On Mon 18-06-18 10:40:14, Tejun Heo wrote:
> On Mon, Jun 18, 2018 at 03:46:58PM +0200, Jan Kara wrote:
> > syzbot is reporting NULL pointer dereference at wb_workfn() [1] due to
> > wb->bdi->dev being NULL. And Dmitry confirmed that wb->state was
> > WB_shutting_down
On Mon 18-06-18 23:38:12, Tetsuo Handa wrote:
> On 2018/06/18 22:46, Jan Kara wrote:
> > syzbot is reporting NULL pointer dereference at wb_workfn() [1] due to
>
> [1]
> https://syzkaller.appspot.com/bug?id=e0818ccb7e46190b3f1038b0c794299208ed4206
>
> line is missing.
&
ase_workfn() using a new mutex. That
way we also no longer need synchronization using WB_shutting_down as the
mutex provides it for CONFIG_CGROUP_WRITEBACK case and without
CONFIG_CGROUP_WRITEBACK wb_shutdown() can be called only once from
bdi_unregister().
Reported-by: syzbot
Signed-off-by: Jan
On Mon 11-06-18 09:01:31, Tejun Heo wrote:
> Hello,
>
> On Mon, Jun 11, 2018 at 11:12:48AM +0200, Jan Kara wrote:
> > However this is wrong and so is the patch. The problem is in
> > cgwb_bdi_unregister() which does cgwb_kill() and thus drops bdi's
> > reference to
is the most robust way.
What do you think about attached patch Tejun? So far only compile tested...
Possible problem with it is that now cgwb_bdi_unregister() will wait for
all wb references to be dropped so it adds some implicit dependencies to
bdi shutdown path.
On Sun 27-05-18 09:47:54, Tetsuo Handa wrote:
> Forwarding
> http://lkml.kernel.org/r/201805251915.fgh64517.hvfjoolffmq...@i-love.sakura.ne.jp
> .
>
> Jan Kara wrote:
> > > void delayed_work_timer_fn(struct timer_list *t)
> > > {
> > > struct delaye
On Sat 19-05-18 23:27:09, Tetsuo Handa wrote:
> Tetsuo Handa wrote:
> > Jan Kara wrote:
> > > Make wb_workfn() use wakeup_wb() for requeueing the work which takes all
> > > the necessary precautions against racing with bdi unregistration.
> >
> >
On Thu 03-05-18 10:48:20, Matthew Wilcox wrote:
> On Thu, May 03, 2018 at 12:05:14PM -0400, Jeff Layton wrote:
> > On Thu, 2018-05-03 at 16:42 +0200, Jan Kara wrote:
> > > On Wed 25-04-18 17:07:48, Fabiano Rosas wrote:
> > > > I'm looking into an issue where removing
;gre...@linuxfoundation.org>
> Cc: Jens Axboe <ax...@kernel.dk>
Looks good to me. You can add:
Reviewed-by: Jan Kara <j...@suse.cz>
Honza
> ---
> drivers/block/loop.c | 11 ++-
> drivers/block/loop
On Thu 03-05-18 18:26:26, Jan Kara wrote:
> Syzbot has reported that it can hit a NULL pointer dereference in
> wb_workfn() due to wb->bdi->dev being NULL. This indicates that
> wb_workfn() was called for an already unregistered bdi which should not
> happen as wb_shut
On Fri 04-05-18 07:55:58, Dave Chinner wrote:
> On Thu, May 03, 2018 at 06:26:26PM +0200, Jan Kara wrote:
> > Syzbot has reported that it can hit a NULL pointer dereference in
> > wb_workfn() due to wb->bdi->dev being NULL. This indicates that
> > wb_workfn() was called
On Fri 04-05-18 07:35:34, Tetsuo Handa wrote:
> Jan Kara wrote:
> > Make wb_workfn() use wakeup_wb() for requeueing the work which takes all
> > the necessary precautions against racing with bdi unregistration.
>
> Yes, this patch will solve NULL pointer dereference bug. Bu
<t...@kernel.org>
Fixes: 839a8e8660b6777e7fe4e80af1a048aebe2b5977
Reported-by: syzbot <syzbot+9873874c735f2892e...@syzkaller.appspotmail.com>
Signed-off-by: Jan Kara <j...@suse.cz>
---
fs/fs-writeback.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/fs/fs-writeback.c b/fs/fs-wr
5318ce7d46866e1d ("bdi:
> Shutdown writeback on all cgwbs in cgwb_bdi_destroy()"). It uses clear_bit()
> to clear WB_shutting_down bit so that threads waiting at wait_on_bit() will
> wake up. But clear_bit() itself does not wake up threads, does it? Who wakes
> them up (e.g. by calling wake_up_bit()) after clear_bit() was called?
Yeah, that's a bug. Thanks for fixing it.
Honza
--
Jan Kara <j...@suse.com>
SUSE Labs, CR
r...@i-love.sakura.ne.jp>
> Reported-by: syzbot <syzbot+c0cf869505e03bdf1...@syzkaller.appspotmail.com>
> Fixes: 5318ce7d46866e1d ("bdi: Shutdown writeback on all cgwbs in
> cgwb_bdi_destroy()")
> Cc: Tejun Heo <t...@kernel.org>
> Cc: Jan Kara <
ct super_block *sb)
> */
> int thaw_super(struct super_block *sb)
> {
> + int error;
> +
> down_write(>s_umount);
> - return thaw_super_locked(sb);
> + error = thaw_super_locked(sb);
> + if (error) {
> + up_write(>s_umount);
> + goto out;
> + }
> +
> + deactivate_locked_super(sb);
> +
> +out:
> + return error;
> }
> EXPORT_SYMBOL(thaw_super);
> --
> 2.16.3
>
--
Jan Kara <j...@suse.com>
SUSE Labs, CR
t; superblock with bd_fsfreeze_count always positive.
>
> Fixes: fcccf502540e3 ("filesystem freeze: implement generic freeze feature")
> Signed-off-by: Luis R. Rodriguez <mcg...@kernel.org>
Looks good to me. You can add:
Reviewed-by: Jan Kara <j...@suse.cz>
lt;mcg...@kernel.org>
Looks good (modulo the -- which is probably worth fixing when touching the
comment anyway). You can add:
Reviewed-by: Jan Kara <j...@suse.cz>
Honza
> ---
> fs/super.c | 12 ++--
>
hat I think we should do is that we'll prevent dirtying of new pages when
we know the underlying device is gone. Because that will fix your problem
and also make sure user sees the IO errors directly instead of just in the
kernel log. The question is how to make this happen in the least painful
way. I think we could intercept writes in grab_cache_page_write_begin()
(which however requires that function to return a proper error code and not
just NULL / non-NULL). And we should also intercept write faults to not
allow page dirtying via mmap - probably somewhere in do_shared_fault() and
do_wp_page(). I've added Jeff to CC since he's dealing with IO error
handling a lot these days. Jeff, what do you think?
Honza
--
Jan Kara <j...@suse.com>
SUSE Labs, CR
-Original Message-
> From: Dave Chinner <da...@fromorbit.com>
> Sent: Tuesday, May 1, 2018 9:46 PM
> To: Jan Kara <j...@suse.cz>
> Cc: linux-...@vger.kernel.org; linux-fsde...@vger.kernel.org;
> linux-block@vger.kernel.org; h...@lst.de; Robert Dorr <rd...@microsof
On Wed 02-05-18 12:45:40, Dave Chinner wrote:
> On Sat, Apr 21, 2018 at 03:03:09PM +0200, Jan Kara wrote:
> > On Wed 18-04-18 14:08:26, Dave Chinner wrote:
> > > From: Dave Chinner <dchin...@redhat.com>
> > >
> > > Currently iomap_dio_rw() only handles
On Wed 25-04-18 00:07:07, Holger Hoffstätte wrote:
> On 04/24/18 19:34, Christoph Hellwig wrote:
> > On Sat, Apr 21, 2018 at 02:54:05PM +0200, Jan Kara wrote:
> > > > - if (iocb->ki_flags & IOCB_DSYNC)
> > > > +
On Fri 20-04-18 11:49:32, Luis R. Rodriguez wrote:
> On Tue, Apr 17, 2018 at 05:59:36PM -0700, Luis R. Rodriguez wrote:
> > On Thu, Dec 21, 2017 at 12:03:29PM +0100, Jan Kara wrote:
> > > Hello,
> > >
> > > I think I owe you a reply here... Sorry that it
o_end(), hence post-IO data stability updates will no
> long race against operations that serialise via inode_dio_wait()
> such as truncate or hole punch.
>
> Signed-Off-By: Dave Chinner <dchin...@redhat.com>
> Reviewed-by: Christoph Hellwig <h...@lst.de&
1 - 100 of 417 matches
Mail list logo
