fs/buffer.c: grow_buffers: fix the uncorrect check
PATCH 1/1 fs/buffer.c: grow_buffers: fix the uncorrect check
When pgoff_t index is 32bit, sector_t block is 64bit, need check if block number is too big. If block is bigger than (4Gb * PAGE_SIZE), index becomes a wrong value.
Commit e5657933863f43cc6bb76a54d659303dafaa9e58 wants to do this. But it gives an uncorrect check. I think block != index << sizebits is correct. And it can detect such issue above.
--- fs/buffer.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
fs/buffer.c +1 -1
@@ -1081,7 +1081,7 @@ grow_buffers(struct block_device *bdev, sector_t block, int size)* Check for a block which wants to lie outside our maximum possible * pagecache index. (this comparison is done using sector_t types). */ -if (unlikely (index != block >> sizebits)) { +if (unlikely (block != index << sizebits)) {char b[BDEVNAME_SIZE ]; printk(KERN_ERR "%s: requested out-of-range block %llu for "
Signature · 1 line
1.9.1
Quoted 4 lines · mnipxh
When
pgoff_tindex is 32bit,sector_tblock is 64bit, need check if block number is too big. If block is bigger than (4Gb *PAGE_SIZE), index becomes a wrong value.
Commite5657933863f43cc6bb76a54d659303dafaa9e58wants to do this. But it gives an uncorrect check. I think block != index << sizebits is correct. And it can detect such issue above.
I don't understand why you think the current check is incorrect.
It is testing that the calculation hasn't overflowed. It is correct. It *should* use the same calculation, to (a) make it more obvious that it's double-checking the earlier calculation in a different type, and
(b) to make it easier for the compiler to optimize it away if sector_t and pgoff_t are the same size.
So the current code is correct, afaik. Note that "index" is "pgoff_t", but "block >> sizebits" is "sector_t".
Linus