| Message ID | 20190729191822.16417-1-mfo@canonical.com |
|---|---|
| State | New |
| Headers | show |
| Series | [D] bcache: only set BCACHE_DEV_WB_RUNNING when cached device attached | expand |
On 7/29/19 12:18 PM, Mauricio Faria de Oliveira wrote: > From: Coly Li <colyli@suse.de> > > BugLink: https://bugs.launchpad.net/bugs/1837788 > > When people set a writeback percent via sysfs file, > /sys/block/bcache<N>/bcache/writeback_percent > current code directly sets BCACHE_DEV_WB_RUNNING to dc->disk.flags > and schedules kworker dc->writeback_rate_update. > > If there is no cache set attached to, the writeback kernel thread is > not running indeed, running dc->writeback_rate_update does not make > sense and may cause NULL pointer deference when reference cache set > pointer inside update_writeback_rate(). > > This patch checks whether the cache set point (dc->disk.c) is NULL in > sysfs interface handler, and only set BCACHE_DEV_WB_RUNNING and > schedule dc->writeback_rate_update when dc->disk.c is not NULL (it > means the cache device is attached to a cache set). > > This problem might be introduced from initial bcache commit, but > commit 3fd47bfe55b0 ("bcache: stop dc->writeback_rate_update properly") > changes part of the original code piece, so I add 'Fixes: 3fd47bfe55b0' > to indicate from which commit this patch can be applied. > > Fixes: 3fd47bfe55b0 ("bcache: stop dc->writeback_rate_update properly") > Reported-by: Bjørn Forsman <bjorn.forsman@gmail.com> > Signed-off-by: Coly Li <colyli@suse.de> > Reviewed-by: Bjørn Forsman <bjorn.forsman@gmail.com> > Cc: stable@vger.kernel.org > Signed-off-by: Jens Axboe <axboe@kernel.dk> > (cherry picked from commit 1f0ffa67349c56ea54c03ccfd1e073c990e7411e) > Signed-off-by: Mauricio Faria de Oliveira <mfo@canonical.com> Acked-by: Connor Kuehl <connor.kuehl@canonical.com> > --- > drivers/md/bcache/sysfs.c | 7 ++++++- > 1 file changed, 6 insertions(+), 1 deletion(-) > > diff --git a/drivers/md/bcache/sysfs.c b/drivers/md/bcache/sysfs.c > index e5daf91310f6..27f78fe98708 100644 > --- a/drivers/md/bcache/sysfs.c > +++ b/drivers/md/bcache/sysfs.c > @@ -414,8 +414,13 @@ STORE(bch_cached_dev) > bch_writeback_queue(dc); > } > > + /* > + * Only set BCACHE_DEV_WB_RUNNING when cached device attached to > + * a cache set, otherwise it doesn't make sense. > + */ > if (attr == &sysfs_writeback_percent) > - if (!test_and_set_bit(BCACHE_DEV_WB_RUNNING, &dc->disk.flags)) > + if ((dc->disk.c != NULL) && > + (!test_and_set_bit(BCACHE_DEV_WB_RUNNING, &dc->disk.flags))) > schedule_delayed_work(&dc->writeback_rate_update, > dc->writeback_rate_update_seconds * HZ); > >
On Mon, Jul 29, 2019 at 04:18:22PM -0300, Mauricio Faria de Oliveira wrote: > From: Coly Li <colyli@suse.de> > > BugLink: https://bugs.launchpad.net/bugs/1837788 > > When people set a writeback percent via sysfs file, > /sys/block/bcache<N>/bcache/writeback_percent > current code directly sets BCACHE_DEV_WB_RUNNING to dc->disk.flags > and schedules kworker dc->writeback_rate_update. > > If there is no cache set attached to, the writeback kernel thread is > not running indeed, running dc->writeback_rate_update does not make > sense and may cause NULL pointer deference when reference cache set > pointer inside update_writeback_rate(). > > This patch checks whether the cache set point (dc->disk.c) is NULL in > sysfs interface handler, and only set BCACHE_DEV_WB_RUNNING and > schedule dc->writeback_rate_update when dc->disk.c is not NULL (it > means the cache device is attached to a cache set). > > This problem might be introduced from initial bcache commit, but > commit 3fd47bfe55b0 ("bcache: stop dc->writeback_rate_update properly") > changes part of the original code piece, so I add 'Fixes: 3fd47bfe55b0' > to indicate from which commit this patch can be applied. > > Fixes: 3fd47bfe55b0 ("bcache: stop dc->writeback_rate_update properly") > Reported-by: Bjørn Forsman <bjorn.forsman@gmail.com> > Signed-off-by: Coly Li <colyli@suse.de> > Reviewed-by: Bjørn Forsman <bjorn.forsman@gmail.com> > Cc: stable@vger.kernel.org > Signed-off-by: Jens Axboe <axboe@kernel.dk> > (cherry picked from commit 1f0ffa67349c56ea54c03ccfd1e073c990e7411e) > Signed-off-by: Mauricio Faria de Oliveira <mfo@canonical.com> Acked-by: Andrea Righi <andrea.righi@canonical.com> > --- > drivers/md/bcache/sysfs.c | 7 ++++++- > 1 file changed, 6 insertions(+), 1 deletion(-) > > diff --git a/drivers/md/bcache/sysfs.c b/drivers/md/bcache/sysfs.c > index e5daf91310f6..27f78fe98708 100644 > --- a/drivers/md/bcache/sysfs.c > +++ b/drivers/md/bcache/sysfs.c > @@ -414,8 +414,13 @@ STORE(bch_cached_dev) > bch_writeback_queue(dc); > } > > + /* > + * Only set BCACHE_DEV_WB_RUNNING when cached device attached to > + * a cache set, otherwise it doesn't make sense. > + */ > if (attr == &sysfs_writeback_percent) > - if (!test_and_set_bit(BCACHE_DEV_WB_RUNNING, &dc->disk.flags)) > + if ((dc->disk.c != NULL) && > + (!test_and_set_bit(BCACHE_DEV_WB_RUNNING, &dc->disk.flags))) > schedule_delayed_work(&dc->writeback_rate_update, > dc->writeback_rate_update_seconds * HZ); > > -- > 2.17.1 > > > -- > kernel-team mailing list > kernel-team@lists.ubuntu.com > https://lists.ubuntu.com/mailman/listinfo/kernel-team
On 2019-07-29 16:18:22 , Mauricio Faria de Oliveira wrote: > From: Coly Li <colyli@suse.de> > > BugLink: https://bugs.launchpad.net/bugs/1837788 > > When people set a writeback percent via sysfs file, > /sys/block/bcache<N>/bcache/writeback_percent > current code directly sets BCACHE_DEV_WB_RUNNING to dc->disk.flags > and schedules kworker dc->writeback_rate_update. > > If there is no cache set attached to, the writeback kernel thread is > not running indeed, running dc->writeback_rate_update does not make > sense and may cause NULL pointer deference when reference cache set > pointer inside update_writeback_rate(). > > This patch checks whether the cache set point (dc->disk.c) is NULL in > sysfs interface handler, and only set BCACHE_DEV_WB_RUNNING and > schedule dc->writeback_rate_update when dc->disk.c is not NULL (it > means the cache device is attached to a cache set). > > This problem might be introduced from initial bcache commit, but > commit 3fd47bfe55b0 ("bcache: stop dc->writeback_rate_update properly") > changes part of the original code piece, so I add 'Fixes: 3fd47bfe55b0' > to indicate from which commit this patch can be applied. > > Fixes: 3fd47bfe55b0 ("bcache: stop dc->writeback_rate_update properly") > Reported-by: Bjørn Forsman <bjorn.forsman@gmail.com> > Signed-off-by: Coly Li <colyli@suse.de> > Reviewed-by: Bjørn Forsman <bjorn.forsman@gmail.com> > Cc: stable@vger.kernel.org > Signed-off-by: Jens Axboe <axboe@kernel.dk> > (cherry picked from commit 1f0ffa67349c56ea54c03ccfd1e073c990e7411e) > Signed-off-by: Mauricio Faria de Oliveira <mfo@canonical.com> > --- > drivers/md/bcache/sysfs.c | 7 ++++++- > 1 file changed, 6 insertions(+), 1 deletion(-) > > diff --git a/drivers/md/bcache/sysfs.c b/drivers/md/bcache/sysfs.c > index e5daf91310f6..27f78fe98708 100644 > --- a/drivers/md/bcache/sysfs.c > +++ b/drivers/md/bcache/sysfs.c > @@ -414,8 +414,13 @@ STORE(bch_cached_dev) > bch_writeback_queue(dc); > } > > + /* > + * Only set BCACHE_DEV_WB_RUNNING when cached device attached to > + * a cache set, otherwise it doesn't make sense. > + */ > if (attr == &sysfs_writeback_percent) > - if (!test_and_set_bit(BCACHE_DEV_WB_RUNNING, &dc->disk.flags)) > + if ((dc->disk.c != NULL) && > + (!test_and_set_bit(BCACHE_DEV_WB_RUNNING, &dc->disk.flags))) > schedule_delayed_work(&dc->writeback_rate_update, > dc->writeback_rate_update_seconds * HZ); > > -- > 2.17.1 > > > -- > kernel-team mailing list > kernel-team@lists.ubuntu.com > https://lists.ubuntu.com/mailman/listinfo/kernel-team
diff --git a/drivers/md/bcache/sysfs.c b/drivers/md/bcache/sysfs.c index e5daf91310f6..27f78fe98708 100644 --- a/drivers/md/bcache/sysfs.c +++ b/drivers/md/bcache/sysfs.c @@ -414,8 +414,13 @@ STORE(bch_cached_dev) bch_writeback_queue(dc); } + /* + * Only set BCACHE_DEV_WB_RUNNING when cached device attached to + * a cache set, otherwise it doesn't make sense. + */ if (attr == &sysfs_writeback_percent) - if (!test_and_set_bit(BCACHE_DEV_WB_RUNNING, &dc->disk.flags)) + if ((dc->disk.c != NULL) && + (!test_and_set_bit(BCACHE_DEV_WB_RUNNING, &dc->disk.flags))) schedule_delayed_work(&dc->writeback_rate_update, dc->writeback_rate_update_seconds * HZ);