Skip to contentSkip to conversation

block_dev:Fix bug when read/write block-device which is larger than 16TB in 32bit-OS.

RFC In review majianpeng 2 participants 5 messages May 29, 2012 – Jul 27, 2012

Subsystems (3)
Memory management VFS Page cache
lore
Share & tools
lore

majianpeng

author
#0 lore raw

RFC 1/1 block_dev:Fix bug when read/write block-device which is larger than 16TB in 32bit-OS.

The size of block-device is larger than 16TB, and the os is 32bit. If the offset of read/write is larger then 16TB. The index of address_space will overflow and supply data from low offset instead.

when read-operation, in function do_generic_file_read():

Quoted 1 line

index = *ppos >> PAGE_CACHE_SHIFT;

Because the *ppos is larger than 16TB and the index is the type pgoff_t which 32bit in 32bit-OS. So index will overflow.

When write-operation, in function generic_write_checks():

Quoted 6 lines
if (likely(!isblk)) {
		.....
	} else {
#ifdef CONFIG_BLOCK
		loff_t isize;
		if (bdev_read_only(I_BDEV(inode)))
			return -EPERM;
Quoted 8 lines
		isize = i_size_read(inode);
		if (*pos >= isize) {
			if (*count || *pos > isize)
				return -ENOSPC;
		}

		if (*pos + *count > isize)
			*count = isize - *pos;

The code only check size.But continue code:
generic_file_buffered_write-->generic_perform_write-->blkdev_write_begin --->block_write_begin()

Quoted 1 line

pgoff_t index = pos >> PAGE_CACHE_SHIFT;

The index will overflow again.

Although filesystem has a attribute s_maxbytes, the block-device was not create so no affect.

Signed-off-by majianpeng <[email protected]>
---
 fs/block_dev.c |    4 +++-
 mm/filemap.c   |   28 ++++++++++++++++++++++++++++
 2 files changed, 31 insertions(+), 1 deletion(-)
fs/block_dev.c +3 -1
index c2bbe1f..1752c0e 100644
@@ -382,7 +382,9 @@ static loff_t block_llseek(struct file *file, loff_t offset, int origin)
 
 	mutex_lock(&bd_inode->i_mutex);
 	size = i_size_read(bd_inode);
-
+#if BITS_PER_LONG == 32
+	size = min_t(loff_t, size, (loff_t)0xFFFFFFFF * PAGE_CACHE_SIZE - 1);
+#endif
 	retval = -EINVAL;
 	switch (origin) {
 		case SEEK_END:
mm/filemap.c +28 -0
index 79c4b2b..34a15bf 100644
@@ -1373,6 +1373,25 @@ int generic_segment_checks(const struct iovec *iov,
 }
 EXPORT_SYMBOL(generic_segment_checks);
 
+static inline
+int generic_read_block_checks(struct file *file, loff_t *pos, size_t *count)
+{
+	struct inode *inode = file->f_mapping->host;
+	loff_t isize = 0;
+#if BITS_PER_LONG == 32 && defined(CONFIG_BLOCK)
+	isize = min_t(loff_t, i_size_read(inode),
+			(loff_t)0xFFFFFFFF * PAGE_CACHE_SIZE - 1);
+	if (*pos >= isize) {
+		if (*count || *pos > isize)
+			return -ENOSPC;
+	}
+
+	if (*pos + *count > isize)
+		*count = isize - *pos;
+#endif
+	return 0;
+}
+
 /**
  * generic_file_aio_read - generic filesystem read routine
  * @iocb:	kernel I/O control block

@@ -1398,6 +1417,11 @@ generic_file_aio_read(struct kiocb *iocb, const struct iovec *iov,
 	if (retval)
 		return retval;
 
+	if (S_ISBLK(filp->f_mapping->host->i_mode)) {
+		retval = generic_read_block_checks(filp, &pos, &count);
+		if (retval)
+			return retval;
+	}
 	/* coalesce the iovecs and go direct-to-BIO for O_DIRECT */
 	if (filp->f_flags & O_DIRECT) {
 		loff_t size;

@@ -2214,6 +2238,10 @@ inline int generic_write_checks(struct file *file, loff_t *pos, size_t *count, i
 		if (bdev_read_only(I_BDEV(inode)))
 			return -EPERM;
 		isize = i_size_read(inode);
+#if BITS_PER_LONG == 32
+		isize = min_t(loff_t, isize,
+				(loff_t)0xFFFFFFFF * PAGE_CACHE_SIZE - 1);
+#endif
 		if (*pos >= isize) {
 			if (*count || *pos > isize)
 				return -ENOSPC;
Signature · 13 lines
1.7.9.5

 				
--------------
majianpeng
2012-05-29

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to [email protected].  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Fight unfair telecom internet charges in Canada: sign http://stopthemeter.ca/
Don't email: <a href=mailto:"[email protected]"> [email protected] </a>
author
#1 lore raw

T24gMjAxMi0wNS0yOSAxNjo1NiBtYWppYW5wZW5nIDxtYWppYW5wZW5nQGdtYWlsLmNvbT4gV3Jv dGU6DQo+VGhlIHNpemUgb2YgYmxvY2stZGV2aWNlIGlzIGxhcmdlciB0aGFuIDE2VEIsIGFuZCB0 aGUgb3MgaXMgMzJiaXQuDQo+SWYgdGhlIG9mZnNldCBvZiByZWFkL3dyaXRlIGlzIGxhcmdlciB0 aGVuIDE2VEIuIFRoZSBpbmRleCBvZiBhZGRyZXNzX3NwYWNlIHdpbGwNCj5vdmVyZmxvdyBhbmQg c3VwcGx5IGRhdGEgZnJvbSBsb3cgb2Zmc2V0IGluc3RlYWQuDQo+DQo+d2hlbiByZWFkLW9wZXJh dGlvbiwgaW4gZnVuY3Rpb24gZG9fZ2VuZXJpY19maWxlX3JlYWQoKToNCj4+aW5kZXggPSAqcHBv cyA+PiBQQUdFX0NBQ0hFX1NISUZUOw0KPkJlY2F1c2UgdGhlICpwcG9zIGlzIGxhcmdlciB0aGFu IDE2VEIgYW5kIHRoZSBpbmRleCAgaXMgdGhlIHR5cGUgcGdvZmZfdCB3aGljaCAzMmJpdA0KPmlu IDMyYml0LU9TLiBTbyBpbmRleCB3aWxsIG92ZXJmbG93Lg0KPg0KPldoZW4gd3JpdGUtb3BlcmF0 aW9uLCBpbiBmdW5jdGlvbiBnZW5lcmljX3dyaXRlX2NoZWNrcygpOg0KPj5pZiAobGlrZWx5KCFp c2JsaykpIHsNCj4+CQkuLi4uLg0KPj4JfSBlbHNlIHsNCj4+I2lmZGVmIENPTkZJR19CTE9DSw0K Pj4JCWxvZmZfdCBpc2l6ZTsNCj4+CQlpZiAoYmRldl9yZWFkX29ubHkoSV9CREVWKGlub2RlKSkp DQo+CQkJcmV0dXJuIC1FUEVSTTsNCj4+CQlpc2l6ZSA9IGlfc2l6ZV9yZWFkKGlub2RlKTsNCj4+ CQlpZiAoKnBvcyA+PSBpc2l6ZSkgew0KPj4JCQlpZiAoKmNvdW50IHx8ICpwb3MgPiBpc2l6ZSkN Cj4+CQkJCXJldHVybiAtRU5PU1BDOw0KPj4JCX0NCj4+DQo+PgkJaWYgKCpwb3MgKyAqY291bnQg PiBpc2l6ZSkNCj4+CQkJKmNvdW50ID0gaXNpemUgLSAqcG9zOw0KPlRoZSBjb2RlIG9ubHkgY2hl Y2sgc2l6ZS5CdXQgY29udGludWUgY29kZToNCj5nZW5lcmljX2ZpbGVfYnVmZmVyZWRfd3JpdGUt LT5nZW5lcmljX3BlcmZvcm1fd3JpdGUtLT5ibGtkZXZfd3JpdGVfYmVnaW4gDQo+LS0tPmJsb2Nr X3dyaXRlX2JlZ2luKCkNCj4+IHBnb2ZmX3QgaW5kZXggPSBwb3MgPj4gUEFHRV9DQUNIRV9TSElG VDsNCj5UaGUgaW5kZXggd2lsbCBvdmVyZmxvdyBhZ2Fpbi4NCj4NCj5BbHRob3VnaCBmaWxlc3lz dGVtIGhhcyBhIGF0dHJpYnV0ZSBzX21heGJ5dGVzLCB0aGUgYmxvY2stZGV2aWNlIHdhcyBub3Qg Y3JlYXRlIHNvIG5vIGFmZmVjdC4NCj4NCj4NCj5TaWduZWQtb2ZmLWJ5OiBtYWppYW5wZW5nIDxt YWppYW5wZW5nQGdtYWlsLmNvbT4NCj4tLS0NCj4gZnMvYmxvY2tfZGV2LmMgfCAgICA0ICsrKy0N Cj4gbW0vZmlsZW1hcC5jICAgfCAgIDI4ICsrKysrKysrKysrKysrKysrKysrKysrKysrKysNCj4g MiBmaWxlcyBjaGFuZ2VkLCAzMSBpbnNlcnRpb25zKCspLCAxIGRlbGV0aW9uKC0pDQo+DQo+ZGlm ZiAtLWdpdCBhL2ZzL2Jsb2NrX2Rldi5jIGIvZnMvYmxvY2tfZGV2LmMNCj5pbmRleCBjMmJiZTFm Li4xNzUyYzBlIDEwMDY0NA0KPi0tLSBhL2ZzL2Jsb2NrX2Rldi5jDQo+KysrIGIvZnMvYmxvY2tf ZGV2LmMNCj5AQCAtMzgyLDcgKzM4Miw5IEBAIHN0YXRpYyBsb2ZmX3QgYmxvY2tfbGxzZWVrKHN0 cnVjdCBmaWxlICpmaWxlLCBsb2ZmX3Qgb2Zmc2V0LCBpbnQgb3JpZ2luKQ0KPiANCj4gCW11dGV4 X2xvY2soJmJkX2lub2RlLT5pX211dGV4KTsNCj4gCXNpemUgPSBpX3NpemVfcmVhZChiZF9pbm9k ZSk7DQo+LQ0KPisjaWYgQklUU19QRVJfTE9ORyA9PSAzMg0KPisJc2l6ZSA9IG1pbl90KGxvZmZf dCwgc2l6ZSwgKGxvZmZfdCkweEZGRkZGRkZGICogUEFHRV9DQUNIRV9TSVpFIC0gMSk7DQo+KyNl bmRpZg0KPiAJcmV0dmFsID0gLUVJTlZBTDsNCj4gCXN3aXRjaCAob3JpZ2luKSB7DQo+IAkJY2Fz ZSBTRUVLX0VORDoNCj5kaWZmIC0tZ2l0IGEvbW0vZmlsZW1hcC5jIGIvbW0vZmlsZW1hcC5jDQo+ aW5kZXggNzljNGIyYi4uMzRhMTViZiAxMDA2NDQNCj4tLS0gYS9tbS9maWxlbWFwLmMNCj4rKysg Yi9tbS9maWxlbWFwLmMNCj5AQCAtMTM3Myw2ICsxMzczLDI1IEBAIGludCBnZW5lcmljX3NlZ21l bnRfY2hlY2tzKGNvbnN0IHN0cnVjdCBpb3ZlYyAqaW92LA0KPiB9DQo+IEVYUE9SVF9TWU1CT0wo Z2VuZXJpY19zZWdtZW50X2NoZWNrcyk7DQo+IA0KPitzdGF0aWMgaW5saW5lDQo+K2ludCBnZW5l cmljX3JlYWRfYmxvY2tfY2hlY2tzKHN0cnVjdCBmaWxlICpmaWxlLCBsb2ZmX3QgKnBvcywgc2l6 ZV90ICpjb3VudCkNCj4rew0KPisJc3RydWN0IGlub2RlICppbm9kZSA9IGZpbGUtPmZfbWFwcGlu Zy0+aG9zdDsNCj4rCWxvZmZfdCBpc2l6ZSA9IDA7DQo+KyNpZiBCSVRTX1BFUl9MT05HID09IDMy ICYmIGRlZmluZWQoQ09ORklHX0JMT0NLKQ0KPisJaXNpemUgPSBtaW5fdChsb2ZmX3QsIGlfc2l6 ZV9yZWFkKGlub2RlKSwNCj4rCQkJKGxvZmZfdCkweEZGRkZGRkZGICogUEFHRV9DQUNIRV9TSVpF IC0gMSk7DQo+KwlpZiAoKnBvcyA+PSBpc2l6ZSkgew0KPisJCWlmICgqY291bnQgfHwgKnBvcyA+ IGlzaXplKQ0KPisJCQlyZXR1cm4gLUVOT1NQQzsNCj4rCX0NCj4rDQo+KwlpZiAoKnBvcyArICpj b3VudCA+IGlzaXplKQ0KPisJCSpjb3VudCA9IGlzaXplIC0gKnBvczsNCj4rI2VuZGlmDQo+Kwly ZXR1cm4gMDsNCj4rfQ0KPisNCj4gLyoqDQo+ICAqIGdlbmVyaWNfZmlsZV9haW9fcmVhZCAtIGdl bmVyaWMgZmlsZXN5c3RlbSByZWFkIHJvdXRpbmUNCj4gICogQGlvY2I6CWtlcm5lbCBJL08gY29u dHJvbCBibG9jaw0KPkBAIC0xMzk4LDYgKzE0MTcsMTEgQEAgZ2VuZXJpY19maWxlX2Fpb19yZWFk KHN0cnVjdCBraW9jYiAqaW9jYiwgY29uc3Qgc3RydWN0IGlvdmVjICppb3YsDQo+IAlpZiAocmV0 dmFsKQ0KPiAJCXJldHVybiByZXR2YWw7DQo+IA0KPisJaWYgKFNfSVNCTEsoZmlscC0+Zl9tYXBw aW5nLT5ob3N0LT5pX21vZGUpKSB7DQo+KwkJcmV0dmFsID0gZ2VuZXJpY19yZWFkX2Jsb2NrX2No ZWNrcyhmaWxwLCAmcG9zLCAmY291bnQpOw0KPisJCWlmIChyZXR2YWwpDQo+KwkJCXJldHVybiBy ZXR2YWw7DQo+Kwl9DQo+IAkvKiBjb2FsZXNjZSB0aGUgaW92ZWNzIGFuZCBnbyBkaXJlY3QtdG8t QklPIGZvciBPX0RJUkVDVCAqLw0KPiAJaWYgKGZpbHAtPmZfZmxhZ3MgJiBPX0RJUkVDVCkgew0K PiAJCWxvZmZfdCBzaXplOw0KPkBAIC0yMjE0LDYgKzIyMzgsMTAgQEAgaW5saW5lIGludCBnZW5l cmljX3dyaXRlX2NoZWNrcyhzdHJ1Y3QgZmlsZSAqZmlsZSwgbG9mZl90ICpwb3MsIHNpemVfdCAq Y291bnQsIGkNCj4gCQlpZiAoYmRldl9yZWFkX29ubHkoSV9CREVWKGlub2RlKSkpDQo+IAkJCXJl dHVybiAtRVBFUk07DQo+IAkJaXNpemUgPSBpX3NpemVfcmVhZChpbm9kZSk7DQo+KyNpZiBCSVRT X1BFUl9MT05HID09IDMyDQo+KwkJaXNpemUgPSBtaW5fdChsb2ZmX3QsIGlzaXplLA0KPisJCQkJ KGxvZmZfdCkweEZGRkZGRkZGICogUEFHRV9DQUNIRV9TSVpFIC0gMSk7DQo+KyNlbmRpZg0KPiAJ CWlmICgqcG9zID49IGlzaXplKSB7DQo+IAkJCWlmICgqY291bnQgfHwgKnBvcyA+IGlzaXplKQ0K PiAJCQkJcmV0dXJuIC1FTk9TUEM7DQo+LS0gDQo+MS43LjkuNQ0KDQpIb3cgYWJvdXQgdGhpcyBw YXRjaD8gb2sgb3IgZXJyb3IgPw0KTm8gb25lIHRvIHJlcGx5PyBNYXliZSB0aGUgcGF0Y2ggZGlk IG5vIHNlbnNlLg0KDQpUaGFuc2shDQo+DQo+IAkJCQkNCj4tLS0tLS0tLS0tLS0tLQ0KPm1hamlh bnBlbmcNCj4yMDEyLTA1LTI5

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in the body to [email protected]. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"[email protected]"> [email protected] </a>

#2 lore raw
Quoted 4 lines · majianpeng

On 2012-05-29 16:56 majianpeng <[email protected]> Wrote:

Quoted 3 lines

The size of block-device is larger than 16TB, and the os is 32bit. If the offset of read/write is larger then 16TB. The index of address_space will overflow and supply data from low offset instead.

We can't support > 16TB block device on 32-bit systems with 4k page size, just like we can't support files that large.

For filesystems the s_maxbytes limit of MAX_LFS_FILESIZE takes care of that, but it seems like we miss that check for block devices.

The proper fix is to add that check (either via s_maxbytes or by checking MAX_LFS_FILESIZE) to generic_write_checks and generic_file_aio_read (or a block device specific wrapper)

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in the body to [email protected]. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"[email protected]"> [email protected] </a>

author
#3 lore raw

On 2012-07-24 21:48 Christoph Hellwig <[email protected]> Wrote:

Quoted 9 lines · majianpeng
Quoted 4 lines

On 2012-05-29 16:56 majianpeng <[email protected]> Wrote:

Quoted 3 lines

The size of block-device is larger than 16TB, and the os is 32bit. If the offset of read/write is larger then 16TB. The index of address_space will overflow and supply data from low offset instead.

We can't support > 16TB block device on 32-bit systems with 4k page size, just like we can't support files that large.

Quoted 7 lines

For filesystems the s_maxbytes limit of MAX_LFS_FILESIZE takes care of that, but it seems like we miss that check for block devices.

The proper fix is to add that check (either via s_maxbytes or by checking MAX_LFS_FILESIZE) to generic_write_checks and generic_file_aio_read (or a block device specific wrapper)

I had a problem:why do read-operation not to check like generic_write_chekcs?

author
#4 lore raw
In reply to Christoph Hellwig

T24gMjAxMi0wNy0yNCAyMTo0OCBDaHJpc3RvcGggSGVsbHdpZyA8aGNoQGluZnJhZGVhZC5vcmc+ IFdyb3RlOg0KPk9uIFR1ZSwgSnVsIDI0LCAyMDEyIGF0IDA4OjQ0OjI3UE0gKzA4MDAsIG1hamlh bnBlbmcgd3JvdGU6DQo+PiBPbiAyMDEyLTA1LTI5IDE2OjU2IG1hamlhbnBlbmcgPG1hamlhbnBl bmdAZ21haWwuY29tPiBXcm90ZToNCj4+ID5UaGUgc2l6ZSBvZiBibG9jay1kZXZpY2UgaXMgbGFy Z2VyIHRoYW4gMTZUQiwgYW5kIHRoZSBvcyBpcyAzMmJpdC4NCj4+ID5JZiB0aGUgb2Zmc2V0IG9m IHJlYWQvd3JpdGUgaXMgbGFyZ2VyIHRoZW4gMTZUQi4gVGhlIGluZGV4IG9mIGFkZHJlc3Nfc3Bh Y2Ugd2lsbA0KPj4gPm92ZXJmbG93IGFuZCBzdXBwbHkgZGF0YSBmcm9tIGxvdyBvZmZzZXQgaW5z dGVhZC4NCj4NCj5XZSBjYW4ndCBzdXBwb3J0ID4gMTZUQiBibG9jayBkZXZpY2Ugb24gMzItYml0 IHN5c3RlbXMgd2l0aCA0ayBwYWdlDQo+c2l6ZSwganVzdCBsaWtlIHdlIGNhbid0IHN1cHBvcnQg ZmlsZXMgdGhhdCBsYXJnZS4NCj4NCj5Gb3IgZmlsZXN5c3RlbXMgdGhlIHNfbWF4Ynl0ZXMgbGlt aXQgb2YgTUFYX0xGU19GSUxFU0laRSB0YWtlcyBjYXJlIG9mDQo+dGhhdCwgYnV0IGl0IHNlZW1z IGxpa2Ugd2UgbWlzcyB0aGF0IGNoZWNrIGZvciBibG9jayBkZXZpY2VzLg0KPg0KPlRoZSBwcm9w ZXIgZml4IGlzIHRvIGFkZCB0aGF0IGNoZWNrIChlaXRoZXIgdmlhIHNfbWF4Ynl0ZXMgb3IgYnkN Cj5jaGVja2luZyBNQVhfTEZTX0ZJTEVTSVpFKSB0byBnZW5lcmljX3dyaXRlX2NoZWNrcyBhbmQN Cj5nZW5lcmljX2ZpbGVfYWlvX3JlYWQgKG9yIGEgYmxvY2sgZGV2aWNlIHNwZWNpZmljIHdyYXBw ZXIpDQo+DQovKiBQYWdlIGNhY2hlIGxpbWl0LiBUaGUgZmlsZXN5c3RlbXMgc2hvdWxkIHB1dCB0 aGF0IGludG8gdGhlaXIgc19tYXhieXRlcyANCiAgIGxpbWl0cywgb3RoZXJ3aXNlIGJhZCB0aGlu Z3MgY2FuIGhhcHBlbiBpbiBWTS4gKi8gDQojaWYgQklUU19QRVJfTE9ORz09MzINCiNkZWZpbmUg TUFYX0xGU19GSUxFU0laRQkoKCh1NjQpUEFHRV9DQUNIRV9TSVpFIDw8IChCSVRTX1BFUl9MT05H LTEpKS0xKSANCiNlbGlmIEJJVFNfUEVSX0xPTkc9PTY0DQojZGVmaW5lIE1BWF9MRlNfRklMRVNJ WkUgCTB4N2ZmZmZmZmZmZmZmZmZmZlVMDQojZW5kaWYNCg0KSWYgd2UgdXNlZCBNQVhfTEZTX0ZJ TEVTSVpFIHRvIGxpbWl0IHRoZSBibG9jay1kZXZpY2UsIHNvIGluIDMyYml0LW9zLCB0aGUgc2l6 ZSBvZiBibG9jayBpcw0Kb25seSA4VCAtMS4NCkJ1dCBpbiBmdW5jdGlvbiBkb19nZW5lcmljX2Zp bGVfcmVhZCgpOg0KPj5pbmRleCA9ICpwcG9zID4+IFBBR0VfQ0FDSEVfU0hJRlQ7DQppbmRleCBp cyB1bnNpZ25lZCBsb25nIHR5cGUuIFNvIHRoZSBwcG9zIGNhbiAxNlQgLTEuDQoNCkJ1dCB0aGUg Y29tbWVudCBzYWlkOg0KPj4vKiBQYWdlIGNhY2hlIGxpbWl0LiBUaGUgZmlsZXN5c3RlbXMgc2hv dWxkIHB1dCB0aGF0IGludG8gdGhlaXIgc19tYXhieXRlcyANCj4+ICAgbGltaXRzLCBvdGhlcndp c2UgYmFkIHRoaW5ncyBjYW4gaGFwcGVuIGluIFZNLiAqLyANCldoeSA/DQoNClRoYW5rcyAhDQo=

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in the body to [email protected]. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"[email protected]"> [email protected] </a>