Hi Dan，
在 2020/6/16 18:20, Dan Carpenter 写道:
On Tue, Jun 16, 2020 at 11:40:02AM +0800, Jason Yan wrote:
Fixes: e525fd89d380 ("block: make blkdev_get/put() handle exclusive access")
I still don't understand how this is the correct fixes tag... :/
git show e525fd89d380:fs/block_dev.c | cat -n
1208 int blkdev_get(struct block_device *bdev, fmode_t mode, void *holder)
1209 {
1210 struct block_device *whole = NULL;
1211 int res;
1212
1213 WARN_ON_ONCE((mode & FMODE_EXCL) && !holder);
1214
1215 if ((mode & FMODE_EXCL) && holder) {
1216 whole = bd_start_claiming(bdev, holder);
1217 if (IS_ERR(whole)) {
1218 bdput(bdev);
1219 return PTR_ERR(whole);
1220 }
1221 }
1222
1223 res = __blkdev_get(bdev, mode, 0);
1224
1225 if (whole) {
1226 if (res == 0)
^^^^^^^^
1227 bd_finish_claiming(bdev, whole, holder);
1228 else
1229 bd_abort_claiming(whole, holder);
^^^^^^^^^^^^^
If __blkdev_get() then this doesn't dereference "bdev" so it's not a
use after free bug.
1230 }
1231
1232 return res;
1233 }
So far as I can see the Fixes tag should be what I said earlier.
Fixes: 89e524c04fa9 ("loop: Fix mount(2) failure due to race with LOOP_SET_FD")
I tried kernel before this commit and can still reproduce this issue.
After some digging, at last I found this one:
77ea887e433a "implement in-kernel gendisk events handling"
@@ -1158,9 +1159,10 @@ int blkdev_get(struct block_device *bdev, fmode_t
mode, void *holder)
if (whole) {
/* finish claiming */
+ mutex_lock(&bdev->bd_mutex);
spin_lock(&bdev_lock);
- if (res == 0) {
+ if (!res) {
BUG_ON(!bd_may_claim(bdev, whole, holder));
/*
* Note that for a whole device bd_holders
This commit added accessing bdev->bd_mutex before checking res, which
will cause use-after-free. So I think the fixes tag should be:
Fixes: 77ea887e433a ("implement in-kernel gendisk events handling")
Otherwise the patch looks good to me.
Reviewed-by: Dan Carpenter <dan.carpenter@xxxxxxxxxx>
regards,
dan carpenter
.
