hfsplus: ensure bio requests are not smaller than the hardware sectors

Currently all bio requests are 512 bytes, which may fail for media whose physical sector size is larger than this. Ensure these requests are not smaller than the block device logical block size. BugLink: http://bugs.launchpad.net/bugs/734883 Signed-off-by: Seth Forshee <seth.forshee@canonical.com> Signed-off-by: Christoph Hellwig <hch@lst.de>

hfsplus: ensure bio requests are not smaller than the hardware sectors
Currently all bio requests are 512 bytes, which may fail for media whose physical sector size is larger than this. Ensure these requests are not smaller than the block device logical block size. BugLink: http://bugs.launchpad.net/bugs/734883 Signed-off-by: Seth Forshee <seth.forshee@canonical.com> Signed-off-by: Christoph Hellwig <hch@lst.de>
Seth Forshee · Christoph Hellwig · Eric Lee · Eric Lee
1 parent aac4e4198e
Showing 4 changed files with 101 additions and 42 deletions Side-by-side Diff
fs/hfsplus/hfsplus_fs.h
fs/hfsplus/part_tbl.c
fs/hfsplus/super.c
fs/hfsplus/wrapper.c
@@ -13,6 +13,7 @@
 #include <linux/fs.h>
 #include <linux/mutex.h>
 #include <linux/buffer_head.h>
+#include <linux/blkdev.h>
 #include "hfsplus_raw.h"
  
 #define DBG_BNODE_REFS	0x00000001
  
@@ -110,7 +111,9 @@
 struct hfs_btree;
  
 struct hfsplus_sb_info {
+	void *s_vhdr_buf;
 	struct hfsplus_vh *s_vhdr;
+	void *s_backup_vhdr_buf;
 	struct hfsplus_vh *s_backup_vhdr;
 	struct hfs_btree *ext_tree;
 	struct hfs_btree *cat_tree;
@@ -258,6 +261,15 @@
 	struct hfsplus_cat_key key;
 };
  
+/*
+ * Find minimum acceptible I/O size for an hfsplus sb.
+ */
+static inline unsigned short hfsplus_min_io_size(struct super_block *sb)
+{
+	return max_t(unsigned short, bdev_logical_block_size(sb->s_bdev),
+		     HFSPLUS_SECTOR_SIZE);
+}
+
 #define hfs_btree_open hfsplus_btree_open
 #define hfs_btree_close hfsplus_btree_close
 #define hfs_btree_write hfsplus_btree_write
@@ -436,8 +448,8 @@
 /* wrapper.c */
 int hfsplus_read_wrapper(struct super_block *);
 int hfs_part_find(struct super_block *, sector_t *, sector_t *);
-int hfsplus_submit_bio(struct block_device *bdev, sector_t sector,
-		void *data, int rw);
+int hfsplus_submit_bio(struct super_block *sb, sector_t sector,
+		void *buf, void **data, int rw);
  
 /* time macros */
 #define __hfsp_mt2ut(t)		(be32_to_cpu(t) - 2082844800U)
@@ -88,11 +88,12 @@
 	return -ENOENT;
 }
  
-static int hfs_parse_new_pmap(struct super_block *sb, struct new_pmap *pm,
-		sector_t *part_start, sector_t *part_size)
+static int hfs_parse_new_pmap(struct super_block *sb, void *buf,
+		struct new_pmap *pm, sector_t *part_start, sector_t *part_size)
 {
 	struct hfsplus_sb_info *sbi = HFSPLUS_SB(sb);
 	int size = be32_to_cpu(pm->pmMapBlkCnt);
+	int buf_size = hfsplus_min_io_size(sb);
 	int res;
 	int i = 0;
  
@@ -107,11 +108,14 @@
 		if (++i >= size)
 			return -ENOENT;
  
-		res = hfsplus_submit_bio(sb->s_bdev,
-					 *part_start + HFS_PMAP_BLK + i,
-					 pm, READ);
-		if (res)
-			return res;
+		pm = (struct new_pmap *)((u8 *)pm + HFSPLUS_SECTOR_SIZE);
+		if ((u8 *)pm - (u8 *)buf >= buf_size) {
+			res = hfsplus_submit_bio(sb,
+						 *part_start + HFS_PMAP_BLK + i,
+						 buf, (void **)&pm, READ);
+			if (res)
+				return res;
+		}
 	} while (pm->pmSig == cpu_to_be16(HFS_NEW_PMAP_MAGIC));
  
 	return -ENOENT;
  
  
@@ -124,15 +128,15 @@
 int hfs_part_find(struct super_block *sb,
 		sector_t *part_start, sector_t *part_size)
 {
-	void *data;
+	void *buf, *data;
 	int res;
  
-	data = kmalloc(HFSPLUS_SECTOR_SIZE, GFP_KERNEL);
-	if (!data)
+	buf = kmalloc(hfsplus_min_io_size(sb), GFP_KERNEL);
+	if (!buf)
 		return -ENOMEM;
  
-	res = hfsplus_submit_bio(sb->s_bdev, *part_start + HFS_PMAP_BLK,
-				 data, READ);
+	res = hfsplus_submit_bio(sb, *part_start + HFS_PMAP_BLK,
+				 buf, &data, READ);
 	if (res)
 		goto out;
  
  
@@ -141,14 +145,14 @@
 		res = hfs_parse_old_pmap(sb, data, part_start, part_size);
 		break;
 	case HFS_NEW_PMAP_MAGIC:
-		res = hfs_parse_new_pmap(sb, data, part_start, part_size);
+		res = hfs_parse_new_pmap(sb, buf, data, part_start, part_size);
 		break;
 	default:
 		res = -ENOENT;
 		break;
 	}
 out:
-	kfree(data);
+	kfree(buf);
 	return res;
 }
@@ -203,17 +203,17 @@
 		write_backup = 1;
 	}
  
-	error2 = hfsplus_submit_bio(sb->s_bdev,
+	error2 = hfsplus_submit_bio(sb,
 				   sbi->part_start + HFSPLUS_VOLHEAD_SECTOR,
-				   sbi->s_vhdr, WRITE_SYNC);
+				   sbi->s_vhdr_buf, NULL, WRITE_SYNC);
 	if (!error)
 		error = error2;
 	if (!write_backup)
 		goto out;
  
-	error2 = hfsplus_submit_bio(sb->s_bdev,
+	error2 = hfsplus_submit_bio(sb,
 				  sbi->part_start + sbi->sect_count - 2,
-				  sbi->s_backup_vhdr, WRITE_SYNC);
+				  sbi->s_backup_vhdr_buf, NULL, WRITE_SYNC);
 	if (!error)
 		error2 = error;
 out:
@@ -257,8 +257,8 @@
 	hfs_btree_close(sbi->ext_tree);
 	iput(sbi->alloc_file);
 	iput(sbi->hidden_dir);
-	kfree(sbi->s_vhdr);
-	kfree(sbi->s_backup_vhdr);
+	kfree(sbi->s_vhdr_buf);
+	kfree(sbi->s_backup_vhdr_buf);
 	unload_nls(sbi->nls);
 	kfree(sb->s_fs_info);
 	sb->s_fs_info = NULL;
@@ -31,34 +31,77 @@
 	complete(bio->bi_private);
 }
  
-int hfsplus_submit_bio(struct block_device *bdev, sector_t sector,
-		void *data, int rw)
+/*
+ * hfsplus_submit_bio - Perfrom block I/O
+ * @sb: super block of volume for I/O
+ * @sector: block to read or write, for blocks of HFSPLUS_SECTOR_SIZE bytes
+ * @buf: buffer for I/O
+ * @data: output pointer for location of requested data
+ * @rw: direction of I/O
+ *
+ * The unit of I/O is hfsplus_min_io_size(sb), which may be bigger than
+ * HFSPLUS_SECTOR_SIZE, and @buf must be sized accordingly. On reads
+ * @data will return a pointer to the start of the requested sector,
+ * which may not be the same location as @buf.
+ *
+ * If @sector is not aligned to the bdev logical block size it will
+ * be rounded down. For writes this means that @buf should contain data
+ * that starts at the rounded-down address. As long as the data was
+ * read using hfsplus_submit_bio() and the same buffer is used things
+ * will work correctly.
+ */
+int hfsplus_submit_bio(struct super_block *sb, sector_t sector,
+		void *buf, void **data, int rw)
 {
 	DECLARE_COMPLETION_ONSTACK(wait);
 	struct bio *bio;
 	int ret = 0;
+	unsigned int io_size;
+	loff_t start;
+	int offset;
  
+	/*
+	 * Align sector to hardware sector size and find offset. We
+	 * assume that io_size is a power of two, which _should_
+	 * be true.
+	 */
+	io_size = hfsplus_min_io_size(sb);
+	start = (loff_t)sector << HFSPLUS_SECTOR_SHIFT;
+	offset = start & (io_size - 1);
+	sector &= ~((io_size >> HFSPLUS_SECTOR_SHIFT) - 1);
+
 	bio = bio_alloc(GFP_NOIO, 1);
 	bio->bi_sector = sector;
-	bio->bi_bdev = bdev;
+	bio->bi_bdev = sb->s_bdev;
 	bio->bi_end_io = hfsplus_end_io_sync;
 	bio->bi_private = &wait;
  
-	/*
-	 * We always submit one sector at a time, so bio_add_page must not fail.
-	 */
-	if (bio_add_page(bio, virt_to_page(data), HFSPLUS_SECTOR_SIZE,
-			 offset_in_page(data)) != HFSPLUS_SECTOR_SIZE)
-		BUG();
+	if (!(rw & WRITE) && data)
+		*data = (u8 *)buf + offset;
  
+	while (io_size > 0) {
+		unsigned int page_offset = offset_in_page(buf);
+		unsigned int len = min_t(unsigned int, PAGE_SIZE - page_offset,
+					 io_size);
+
+		ret = bio_add_page(bio, virt_to_page(buf), len, page_offset);
+		if (ret != len) {
+			ret = -EIO;
+			goto out;
+		}
+		io_size -= len;
+		buf = (u8 *)buf + len;
+	}
+
 	submit_bio(rw, bio);
 	wait_for_completion(&wait);
  
 	if (!bio_flagged(bio, BIO_UPTODATE))
 		ret = -EIO;
  
+out:
 	bio_put(bio);
-	return ret;
+	return ret < 0 ? ret : 0;
 }
  
 static int hfsplus_read_mdb(void *bufptr, struct hfsplus_wd *wd)
  
  
@@ -143,17 +186,17 @@
 		goto out;
  
 	error = -ENOMEM;
-	sbi->s_vhdr = kmalloc(HFSPLUS_SECTOR_SIZE, GFP_KERNEL);
-	if (!sbi->s_vhdr)
+	sbi->s_vhdr_buf = kmalloc(hfsplus_min_io_size(sb), GFP_KERNEL);
+	if (!sbi->s_vhdr_buf)
 		goto out;
-	sbi->s_backup_vhdr = kmalloc(HFSPLUS_SECTOR_SIZE, GFP_KERNEL);
-	if (!sbi->s_backup_vhdr)
+	sbi->s_backup_vhdr_buf = kmalloc(hfsplus_min_io_size(sb), GFP_KERNEL);
+	if (!sbi->s_backup_vhdr_buf)
 		goto out_free_vhdr;
  
 reread:
-	error = hfsplus_submit_bio(sb->s_bdev,
-				   part_start + HFSPLUS_VOLHEAD_SECTOR,
-				   sbi->s_vhdr, READ);
+	error = hfsplus_submit_bio(sb, part_start + HFSPLUS_VOLHEAD_SECTOR,
+				   sbi->s_vhdr_buf, (void **)&sbi->s_vhdr,
+				   READ);
 	if (error)
 		goto out_free_backup_vhdr;
  
@@ -183,9 +226,9 @@
 		goto reread;
 	}
  
-	error = hfsplus_submit_bio(sb->s_bdev,
-				   part_start + part_size - 2,
-				   sbi->s_backup_vhdr, READ);
+	error = hfsplus_submit_bio(sb, part_start + part_size - 2,
+				   sbi->s_backup_vhdr_buf,
+				   (void **)&sbi->s_backup_vhdr, READ);
 	if (error)
 		goto out_free_backup_vhdr;
...	...	@@ -13,6 +13,7 @@
13	13	#include <linux/fs.h>
14	14	#include <linux/mutex.h>
15	15	#include <linux/buffer_head.h>
	16	+#include <linux/blkdev.h>
16	17	#include "hfsplus_raw.h"
17	18
18	19	#define DBG_BNODE_REFS 0x00000001
19	20
...	...	@@ -110,7 +111,9 @@
110	111	struct hfs_btree;
111	112
112	113	struct hfsplus_sb_info {
	114	+ void *s_vhdr_buf;
113	115	struct hfsplus_vh *s_vhdr;
	116	+ void *s_backup_vhdr_buf;
114	117	struct hfsplus_vh *s_backup_vhdr;
115	118	struct hfs_btree *ext_tree;
116	119	struct hfs_btree *cat_tree;
...	...	@@ -258,6 +261,15 @@
258	261	struct hfsplus_cat_key key;
259	262	};
260	263
	264	+/*
	265	+ * Find minimum acceptible I/O size for an hfsplus sb.
	266	+ */
	267	+static inline unsigned short hfsplus_min_io_size(struct super_block *sb)
	268	+{
	269	+ return max_t(unsigned short, bdev_logical_block_size(sb->s_bdev),
	270	+ HFSPLUS_SECTOR_SIZE);
	271	+}
	272	+
261	273	#define hfs_btree_open hfsplus_btree_open
262	274	#define hfs_btree_close hfsplus_btree_close
263	275	#define hfs_btree_write hfsplus_btree_write
...	...	@@ -436,8 +448,8 @@
436	448	/* wrapper.c */
437	449	int hfsplus_read_wrapper(struct super_block *);
438	450	int hfs_part_find(struct super_block , sector_t , sector_t *);
439		-int hfsplus_submit_bio(struct block_device *bdev, sector_t sector,
440		- void *data, int rw);
	451	+int hfsplus_submit_bio(struct super_block *sb, sector_t sector,
	452	+ void buf, void *data, int rw);
441	453
442	454	/* time macros */
443	455	#define __hfsp_mt2ut(t) (be32_to_cpu(t) - 2082844800U)
...	...	@@ -88,11 +88,12 @@
88	88	return -ENOENT;
89	89	}
90	90
91		-static int hfs_parse_new_pmap(struct super_block sb, struct new_pmap pm,
92		- sector_t part_start, sector_t part_size)
	91	+static int hfs_parse_new_pmap(struct super_block sb, void buf,
	92	+ struct new_pmap pm, sector_t part_start, sector_t *part_size)
93	93	{
94	94	struct hfsplus_sb_info *sbi = HFSPLUS_SB(sb);
95	95	int size = be32_to_cpu(pm->pmMapBlkCnt);
	96	+ int buf_size = hfsplus_min_io_size(sb);
96	97	int res;
97	98	int i = 0;
98	99
...	...	@@ -107,11 +108,14 @@
107	108	if (++i >= size)
108	109	return -ENOENT;
109	110
110		- res = hfsplus_submit_bio(sb->s_bdev,
111		- *part_start + HFS_PMAP_BLK + i,
112		- pm, READ);
113		- if (res)
114		- return res;
	111	+ pm = (struct new_pmap )((u8 )pm + HFSPLUS_SECTOR_SIZE);
	112	+ if ((u8 )pm - (u8 )buf >= buf_size) {
	113	+ res = hfsplus_submit_bio(sb,
	114	+ *part_start + HFS_PMAP_BLK + i,
	115	+ buf, (void **)&pm, READ);
	116	+ if (res)
	117	+ return res;
	118	+ }
115	119	} while (pm->pmSig == cpu_to_be16(HFS_NEW_PMAP_MAGIC));
116	120
117	121	return -ENOENT;
118	122
119	123
...	...	@@ -124,15 +128,15 @@
124	128	int hfs_part_find(struct super_block *sb,
125	129	sector_t part_start, sector_t part_size)
126	130	{
127		- void *data;
	131	+ void buf, data;
128	132	int res;
129	133
130		- data = kmalloc(HFSPLUS_SECTOR_SIZE, GFP_KERNEL);
131		- if (!data)
	134	+ buf = kmalloc(hfsplus_min_io_size(sb), GFP_KERNEL);
	135	+ if (!buf)
132	136	return -ENOMEM;
133	137
134		- res = hfsplus_submit_bio(sb->s_bdev, *part_start + HFS_PMAP_BLK,
135		- data, READ);
	138	+ res = hfsplus_submit_bio(sb, *part_start + HFS_PMAP_BLK,
	139	+ buf, &data, READ);
136	140	if (res)
137	141	goto out;
138	142
139	143
...	...	@@ -141,14 +145,14 @@
141	145	res = hfs_parse_old_pmap(sb, data, part_start, part_size);
142	146	break;
143	147	case HFS_NEW_PMAP_MAGIC:
144		- res = hfs_parse_new_pmap(sb, data, part_start, part_size);
	148	+ res = hfs_parse_new_pmap(sb, buf, data, part_start, part_size);
145	149	break;
146	150	default:
147	151	res = -ENOENT;
148	152	break;
149	153	}
150	154	out:
151		- kfree(data);
	155	+ kfree(buf);
152	156	return res;
153	157	}
...	...	@@ -203,17 +203,17 @@
203	203	write_backup = 1;
204	204	}
205	205
206		- error2 = hfsplus_submit_bio(sb->s_bdev,
	206	+ error2 = hfsplus_submit_bio(sb,
207	207	sbi->part_start + HFSPLUS_VOLHEAD_SECTOR,
208		- sbi->s_vhdr, WRITE_SYNC);
	208	+ sbi->s_vhdr_buf, NULL, WRITE_SYNC);
209	209	if (!error)
210	210	error = error2;
211	211	if (!write_backup)
212	212	goto out;
213	213
214		- error2 = hfsplus_submit_bio(sb->s_bdev,
	214	+ error2 = hfsplus_submit_bio(sb,
215	215	sbi->part_start + sbi->sect_count - 2,
216		- sbi->s_backup_vhdr, WRITE_SYNC);
	216	+ sbi->s_backup_vhdr_buf, NULL, WRITE_SYNC);
217	217	if (!error)
218	218	error2 = error;
219	219	out:
...	...	@@ -257,8 +257,8 @@
257	257	hfs_btree_close(sbi->ext_tree);
258	258	iput(sbi->alloc_file);
259	259	iput(sbi->hidden_dir);
260		- kfree(sbi->s_vhdr);
261		- kfree(sbi->s_backup_vhdr);
	260	+ kfree(sbi->s_vhdr_buf);
	261	+ kfree(sbi->s_backup_vhdr_buf);
262	262	unload_nls(sbi->nls);
263	263	kfree(sb->s_fs_info);
264	264	sb->s_fs_info = NULL;
...	...	@@ -31,34 +31,77 @@
31	31	complete(bio->bi_private);
32	32	}
33	33
34		-int hfsplus_submit_bio(struct block_device *bdev, sector_t sector,
35		- void *data, int rw)
	34	+/*
	35	+ * hfsplus_submit_bio - Perfrom block I/O
	36	+ * @sb: super block of volume for I/O
	37	+ * @sector: block to read or write, for blocks of HFSPLUS_SECTOR_SIZE bytes
	38	+ * @buf: buffer for I/O
	39	+ * @data: output pointer for location of requested data
	40	+ * @rw: direction of I/O
	41	+ *
	42	+ * The unit of I/O is hfsplus_min_io_size(sb), which may be bigger than
	43	+ * HFSPLUS_SECTOR_SIZE, and @buf must be sized accordingly. On reads
	44	+ * @data will return a pointer to the start of the requested sector,
	45	+ * which may not be the same location as @buf.
	46	+ *
	47	+ * If @sector is not aligned to the bdev logical block size it will
	48	+ * be rounded down. For writes this means that @buf should contain data
	49	+ * that starts at the rounded-down address. As long as the data was
	50	+ * read using hfsplus_submit_bio() and the same buffer is used things
	51	+ * will work correctly.
	52	+ */
	53	+int hfsplus_submit_bio(struct super_block *sb, sector_t sector,
	54	+ void buf, void *data, int rw)
36	55	{
37	56	DECLARE_COMPLETION_ONSTACK(wait);
38	57	struct bio *bio;
39	58	int ret = 0;
	59	+ unsigned int io_size;
	60	+ loff_t start;
	61	+ int offset;
40	62
	63	+ /*
	64	+ * Align sector to hardware sector size and find offset. We
	65	+ * assume that io_size is a power of two, which _should_
	66	+ * be true.
	67	+ */
	68	+ io_size = hfsplus_min_io_size(sb);
	69	+ start = (loff_t)sector << HFSPLUS_SECTOR_SHIFT;
	70	+ offset = start & (io_size - 1);
	71	+ sector &= ~((io_size >> HFSPLUS_SECTOR_SHIFT) - 1);
	72	+
41	73	bio = bio_alloc(GFP_NOIO, 1);
42	74	bio->bi_sector = sector;
43		- bio->bi_bdev = bdev;
	75	+ bio->bi_bdev = sb->s_bdev;
44	76	bio->bi_end_io = hfsplus_end_io_sync;
45	77	bio->bi_private = &wait;
46	78
47		- /*
48		- * We always submit one sector at a time, so bio_add_page must not fail.
49		- */
50		- if (bio_add_page(bio, virt_to_page(data), HFSPLUS_SECTOR_SIZE,
51		- offset_in_page(data)) != HFSPLUS_SECTOR_SIZE)
52		- BUG();
	79	+ if (!(rw & WRITE) && data)
	80	+ data = (u8 )buf + offset;
53	81
	82	+ while (io_size > 0) {
	83	+ unsigned int page_offset = offset_in_page(buf);
	84	+ unsigned int len = min_t(unsigned int, PAGE_SIZE - page_offset,
	85	+ io_size);
	86	+
	87	+ ret = bio_add_page(bio, virt_to_page(buf), len, page_offset);
	88	+ if (ret != len) {
	89	+ ret = -EIO;
	90	+ goto out;
	91	+ }
	92	+ io_size -= len;
	93	+ buf = (u8 *)buf + len;
	94	+ }
	95	+
54	96	submit_bio(rw, bio);
55	97	wait_for_completion(&wait);
56	98
57	99	if (!bio_flagged(bio, BIO_UPTODATE))
58	100	ret = -EIO;
59	101
	102	+out:
60	103	bio_put(bio);
61		- return ret;
	104	+ return ret < 0 ? ret : 0;
62	105	}
63	106
64	107	static int hfsplus_read_mdb(void bufptr, struct hfsplus_wd wd)
65	108
66	109
...	...	@@ -143,17 +186,17 @@
143	186	goto out;
144	187
145	188	error = -ENOMEM;
146		- sbi->s_vhdr = kmalloc(HFSPLUS_SECTOR_SIZE, GFP_KERNEL);
147		- if (!sbi->s_vhdr)
	189	+ sbi->s_vhdr_buf = kmalloc(hfsplus_min_io_size(sb), GFP_KERNEL);
	190	+ if (!sbi->s_vhdr_buf)
148	191	goto out;
149		- sbi->s_backup_vhdr = kmalloc(HFSPLUS_SECTOR_SIZE, GFP_KERNEL);
150		- if (!sbi->s_backup_vhdr)
	192	+ sbi->s_backup_vhdr_buf = kmalloc(hfsplus_min_io_size(sb), GFP_KERNEL);
	193	+ if (!sbi->s_backup_vhdr_buf)
151	194	goto out_free_vhdr;
152	195
153	196	reread:
154		- error = hfsplus_submit_bio(sb->s_bdev,
155		- part_start + HFSPLUS_VOLHEAD_SECTOR,
156		- sbi->s_vhdr, READ);
	197	+ error = hfsplus_submit_bio(sb, part_start + HFSPLUS_VOLHEAD_SECTOR,
	198	+ sbi->s_vhdr_buf, (void **)&sbi->s_vhdr,
	199	+ READ);
157	200	if (error)
158	201	goto out_free_backup_vhdr;
159	202
...	...	@@ -183,9 +226,9 @@
183	226	goto reread;
184	227	}
185	228
186		- error = hfsplus_submit_bio(sb->s_bdev,
187		- part_start + part_size - 2,
188		- sbi->s_backup_vhdr, READ);
	229	+ error = hfsplus_submit_bio(sb, part_start + part_size - 2,
	230	+ sbi->s_backup_vhdr_buf,
	231	+ (void **)&sbi->s_backup_vhdr, READ);
189	232	if (error)
190	233	goto out_free_backup_vhdr;
191	234