On (23/11/07 16:39), Sergey Senozhatsky wrote:
> On (23/11/06 22:54), Vasily Averin wrote:
> > @@ -1362,14 +1362,14 @@ static int zram_read_page(struct zram *zram, struct page *page, u32 index,
> > ret = zram_read_from_zspool(zram, page, index);
> > zram_slot_unlock(zram, index);
> > } else {
> > + unsigned long entry = zram_get_element(zram, index);
> > /*
> > * The slot should be unlocked before reading from the backing
> > * device.
> > */
> > zram_slot_unlock(zram, index);
> >
> > - ret = read_from_bdev(zram, page, zram_get_element(zram, index),
> > - parent);
> > + ret = read_from_bdev(zram, page, entry, parent);
>
> Hmmm,
> We may want to do more here. Basically, we probably need to re-confirm
> after read_from_bdev() that the entry at index still has ZRAM_WB set
> and, if so, that it points to the same blk_idx. IOW, check that it has
> not been free-ed and re-used under us.
>
> Minchan, what do you think? Is that scenario possible?
Basically
---
diff --git a/drivers/block/zram/zram_drv.c b/drivers/block/zram/zram_drv.c
index f5e3756fc21a..2d69f6efcee3 100644
--- a/drivers/block/zram/zram_drv.c
+++ b/drivers/block/zram/zram_drv.c
@@ -1364,14 +1364,21 @@ static int zram_read_page(struct zram *zram, struct page *page, u32 index,
ret = zram_read_from_zspool(zram, page, index);
zram_slot_unlock(zram, index);
} else {
+ unsigned long idx = zram_get_element(zram, index);
/*
* The slot should be unlocked before reading from the backing
* device.
*/
zram_slot_unlock(zram, index);
- ret = read_from_bdev(zram, page, zram_get_element(zram, index),
- parent);
+ ret = read_from_bdev(zram, page, idx, parent);
+ if (ret == 0) {
+ zram_slot_lock(zram, index);
+ if (!zram_test_flag(zram, index, ZRAM_WB) ||
+ idx != zram_get_element(zram, index))
+ ret = -EINVAL;
+ zram_slot_unlock(zram, index);
+ }
}
/* Should NEVER happen. Return bio error if it does. */
