Re: [PATCH 2/2] ext2: Avoid rec_len overflow with 64KB block size
On Wed, 17 Oct 2007, Andrew Morton wrote: b) what happens when an old ext2 driver tries to read and/or write this directory entry? Do we need a compat flag for it? Old ext2 only supports up to 4k include/linux/ext2_fs.h: #define EXT2_MIN_BLOCK_SIZE 1024 #define EXT2_MAX_BLOCK_SIZE 4096 #define EXT2_MIN_BLOCK_LOG_SIZE 10 Should fail to mount the volume since the block size is too large. - To unsubscribe from this list: send the line unsubscribe linux-ext4 in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 2/2] ext2: Avoid rec_len overflow with 64KB block size
On Thu, 18 Oct 2007 02:03:39 -0700 (PDT) Christoph Lameter [EMAIL PROTECTED] wrote: On Wed, 17 Oct 2007, Andrew Morton wrote: b) what happens when an old ext2 driver tries to read and/or write this directory entry? Do we need a compat flag for it? Old ext2 only supports up to 4k include/linux/ext2_fs.h: #define EXT2_MIN_BLOCK_SIZE 1024 #define EXT2_MAX_BLOCK_SIZE 4096 #define EXT2_MIN_BLOCK_LOG_SIZE 10 Should fail to mount the volume since the block size is too large. should, but does it? box:/usr/src/25 grep MAX_BLOCK_SIZE fs/ext2/*.[ch] include/linux/ext2* include/linux/ext2_fs.h:#define EXT2_MAX_BLOCK_SIZE 4096 box:/usr/src/25 - To unsubscribe from this list: send the line unsubscribe linux-ext4 in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 2/2] ext2: Avoid rec_len overflow with 64KB block size
On Wed, 2007-10-17 at 21:09 -0700, Andrew Morton wrote: On Thu, 11 Oct 2007 13:18:49 +0200 Jan Kara [EMAIL PROTECTED] wrote: With 64KB blocksize, a directory entry can have size 64KB which does not fit into 16 bits we have for entry lenght. So we store 0x instead and convert value when read from / written to disk. btw, this changes ext2's on-disk format. Just to clarify this is only changes the directory entries format on ext2/3/4 fs with 64k block size. But currently without kernel changes ext2/3/4 does not support 64 block size. a) is the ext2 format documented anywhere? If so, that document will need updating. The e2fsprogs needs to be changed to sync up with this change. Ted has a paper a while back to show ext2 disk format http://web.mit.edu/tytso/www/linux/ext2intro.html The Documentation/filesystems/ext2.txt doesn't have the ext2 format documented. That document is out-dated need to be reviewed and cleaned up. b) what happens when an old ext2 driver tries to read and/or write this directory entry? Do we need a compat flag for it? c) what happens when old and new ext3 or ext4 try to read/write this directory entry? Without the first patch in this series: ext2 large blocksize support patches, it fails to mount a ext2 filesystem with 64k block size. [PATCH 1/2] ext2: Support large blocksize up to PAGESIZE http://lkml.org/lkml/2007/10/1/361 So the old ext2/3/4 driver will not get access the directory entry with 64k block size format changes. Regards, Mingming - To unsubscribe from this list: send the line unsubscribe linux-ext4 in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html - To unsubscribe from this list: send the line unsubscribe linux-ext4 in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 2/2] ext2: Avoid rec_len overflow with 64KB block size
On Thu, 11 Oct 2007 13:18:49 +0200 Jan Kara [EMAIL PROTECTED] wrote: +static inline __le16 ext2_rec_len_to_disk(unsigned len) +{ + if (len == (1 16)) + return cpu_to_le16(EXT2_MAX_REC_LEN); + else if (len (1 16)) + BUG(); + return cpu_to_le16(len); +} Of course, ext2 shouldn't be trying to write a bad record length into a directory entry. But are we sure that there is no way in which this situation could occur is the on-disk data was _already_ bad? Because it is very bad for a fileysstem to go BUG in response to unexpected data on the disk. - To unsubscribe from this list: send the line unsubscribe linux-ext4 in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 2/2] ext2: Avoid rec_len overflow with 64KB block size
On Thu, 11 Oct 2007 13:18:49 +0200 Jan Kara [EMAIL PROTECTED] wrote: With 64KB blocksize, a directory entry can have size 64KB which does not fit into 16 bits we have for entry lenght. So we store 0x instead and convert value when read from / written to disk. btw, this changes ext2's on-disk format. a) is the ext2 format documented anywhere? If so, that document will need updating. b) what happens when an old ext2 driver tries to read and/or write this directory entry? Do we need a compat flag for it? c) what happens when old and new ext3 or ext4 try to read/write this directory entry? - To unsubscribe from this list: send the line unsubscribe linux-ext4 in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 2/2] ext2: Avoid rec_len overflow with 64KB block size
On Thu 04-10-07 16:11:21, Andrew Morton wrote: On Thu, 4 Oct 2007 16:40:44 -0600 Andreas Dilger [EMAIL PROTECTED] wrote: On Oct 04, 2007 13:12 -0700, Andrew Morton wrote: On Mon, 01 Oct 2007 17:35:46 -0700 ext2: Avoid rec_len overflow with 64KB block size into 16 bits we have for entry lenght. So we store 0x instead and convert value when read from / written to disk. This patch clashes in non-trivial ways with ext2-convert-to-new-aops-fix.patch and perhaps other things which are already queued for 2.6.24 inclusion, so I'll need to ask for an updated patch, please. If the rel_len overflow patch isn't going to make it, then we also need to revert the EXT*_MAX_BLOCK_SIZE change to 65536. It would be possible to allow this to be up to 32768 w/o the rec_len overflow fix however. Ok, thanks, I dropped ext3-support-large-blocksize-up-to-pagesize.patch and ext2-support-large-blocksize-up-to-pagesize.patch. Sorry, for the delayed answer but I had some urgent bugs to fix... Why did you drom ext3-support-large-blocksize-up-to-pagesize.patch? As far as I understand your previous email (and also as I've checked against 2.6.23-rc8-mm2), the patch fixing rec_len overflow clashes only for ext2... I'll send you an updated patch for ext2 in a moment. Honza -- Jan Kara [EMAIL PROTECTED] SUSE Labs, CR - To unsubscribe from this list: send the line unsubscribe linux-ext4 in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 2/2] ext2: Avoid rec_len overflow with 64KB block size
On Thu 04-10-07 13:12:07, Andrew Morton wrote: On Mon, 01 Oct 2007 17:35:46 -0700 Mingming Cao [EMAIL PROTECTED] wrote: ext2: Avoid rec_len overflow with 64KB block size From: Jan Kara [EMAIL PROTECTED] With 64KB blocksize, a directory entry can have size 64KB which does not fit into 16 bits we have for entry lenght. So we store 0x instead and convert value when read from / written to disk. This patch clashes in non-trivial ways with ext2-convert-to-new-aops-fix.patch and perhaps other things which are already queued for 2.6.24 inclusion, so I'll need to ask for an updated patch, please. Also, I'm planing on merging the ext2 reservations code into 2.6.24, so if we're aiming for complete support of 64k blocksize in 2.6.24's ext2, additional testing and checking will be needed. OK, attached is a patch diffed against 2.6.23-rc9-mm2 - does that work fine for you? Honza -- Jan Kara [EMAIL PROTECTED] SUSE Labs, CR -- With 64KB blocksize, a directory entry can have size 64KB which does not fit into 16 bits we have for entry lenght. So we store 0x instead and convert value when read from / written to disk. Signed-off-by: Jan Kara [EMAIL PROTECTED] diff -rupX /home/jack/.kerndiffexclude linux-2.6.23-mm/fs/ext2/dir.c linux-2.6.23-mm-1-ext2_64k_rec_len/fs/ext2/dir.c --- linux-2.6.23-mm/fs/ext2/dir.c 2007-10-11 12:08:16.0 +0200 +++ linux-2.6.23-mm-1-ext2_64k_rec_len/fs/ext2/dir.c2007-10-11 12:14:24.0 +0200 @@ -28,6 +28,24 @@ typedef struct ext2_dir_entry_2 ext2_dirent; +static inline unsigned ext2_rec_len_from_disk(__le16 dlen) +{ + unsigned len = le16_to_cpu(dlen); + + if (len == EXT2_MAX_REC_LEN) + return 1 16; + return len; +} + +static inline __le16 ext2_rec_len_to_disk(unsigned len) +{ + if (len == (1 16)) + return cpu_to_le16(EXT2_MAX_REC_LEN); + else if (len (1 16)) + BUG(); + return cpu_to_le16(len); +} + /* * ext2 uses block-sized chunks. Arguably, sector-sized ones would be * more robust, but we have what we have @@ -106,7 +124,7 @@ static void ext2_check_page(struct page } for (offs = 0; offs = limit - EXT2_DIR_REC_LEN(1); offs += rec_len) { p = (ext2_dirent *)(kaddr + offs); - rec_len = le16_to_cpu(p-rec_len); + rec_len = ext2_rec_len_from_disk(p-rec_len); if (rec_len EXT2_DIR_REC_LEN(1)) goto Eshort; @@ -204,7 +222,8 @@ static inline int ext2_match (int len, c */ static inline ext2_dirent *ext2_next_entry(ext2_dirent *p) { - return (ext2_dirent *)((char*)p + le16_to_cpu(p-rec_len)); + return (ext2_dirent *)((char*)p + + ext2_rec_len_from_disk(p-rec_len)); } static inline unsigned @@ -316,7 +335,7 @@ ext2_readdir (struct file * filp, void * return 0; } } - filp-f_pos += le16_to_cpu(de-rec_len); + filp-f_pos += ext2_rec_len_from_disk(de-rec_len); } ext2_put_page(page); } @@ -425,7 +444,7 @@ void ext2_set_link(struct inode *dir, st { loff_t pos = page_offset(page) + (char *) de - (char *) page_address(page); - unsigned len = le16_to_cpu(de-rec_len); + unsigned len = ext2_rec_len_from_disk(de-rec_len); int err; lock_page(page); @@ -482,7 +501,7 @@ int ext2_add_link (struct dentry *dentry /* We hit i_size */ name_len = 0; rec_len = chunk_size; - de-rec_len = cpu_to_le16(chunk_size); + de-rec_len = ext2_rec_len_to_disk(chunk_size); de-inode = 0; goto got_it; } @@ -496,7 +515,7 @@ int ext2_add_link (struct dentry *dentry if (ext2_match (namelen, name, de)) goto out_unlock; name_len = EXT2_DIR_REC_LEN(de-name_len); - rec_len = le16_to_cpu(de-rec_len); + rec_len = ext2_rec_len_from_disk(de-rec_len); if (!de-inode rec_len = reclen) goto got_it; if (rec_len = name_len + reclen) @@ -518,8 +537,8 @@ got_it: goto out_unlock; if (de-inode) { ext2_dirent *de1 = (ext2_dirent *) ((char *) de + name_len); - de1-rec_len = cpu_to_le16(rec_len - name_len); - de-rec_len = cpu_to_le16(name_len); + de1-rec_len = ext2_rec_len_to_disk(rec_len - name_len); + de-rec_len =
Re: [PATCH 2/2] ext2: Avoid rec_len overflow with 64KB block size
On Thu 04-10-07 13:12:07, Andrew Morton wrote: On Mon, 01 Oct 2007 17:35:46 -0700 Mingming Cao [EMAIL PROTECTED] wrote: ext2: Avoid rec_len overflow with 64KB block size From: Jan Kara [EMAIL PROTECTED] With 64KB blocksize, a directory entry can have size 64KB which does not fit into 16 bits we have for entry lenght. So we store 0x instead and convert value when read from / written to disk. This patch clashes in non-trivial ways with ext2-convert-to-new-aops-fix.patch and perhaps other things which are already queued for 2.6.24 inclusion, so I'll need to ask for an updated patch, please. Also, I'm planing on merging the ext2 reservations code into 2.6.24, so if we're aiming for complete support of 64k blocksize in 2.6.24's ext2, additional testing and checking will be needed. OK, I'll fixup those rejects and send a new patch. Honza -- Jan Kara [EMAIL PROTECTED] SUSE Labs, CR - To unsubscribe from this list: send the line unsubscribe linux-ext4 in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 2/2] ext2: Avoid rec_len overflow with 64KB block size
On Mon, 01 Oct 2007 17:35:46 -0700 Mingming Cao [EMAIL PROTECTED] wrote: ext2: Avoid rec_len overflow with 64KB block size From: Jan Kara [EMAIL PROTECTED] With 64KB blocksize, a directory entry can have size 64KB which does not fit into 16 bits we have for entry lenght. So we store 0x instead and convert value when read from / written to disk. This patch clashes in non-trivial ways with ext2-convert-to-new-aops-fix.patch and perhaps other things which are already queued for 2.6.24 inclusion, so I'll need to ask for an updated patch, please. Also, I'm planing on merging the ext2 reservations code into 2.6.24, so if we're aiming for complete support of 64k blocksize in 2.6.24's ext2, additional testing and checking will be needed. - To unsubscribe from this list: send the line unsubscribe linux-ext4 in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 2/2] ext2: Avoid rec_len overflow with 64KB block size
On Thu, 4 Oct 2007 16:40:44 -0600 Andreas Dilger [EMAIL PROTECTED] wrote: On Oct 04, 2007 13:12 -0700, Andrew Morton wrote: On Mon, 01 Oct 2007 17:35:46 -0700 ext2: Avoid rec_len overflow with 64KB block size into 16 bits we have for entry lenght. So we store 0x instead and convert value when read from / written to disk. This patch clashes in non-trivial ways with ext2-convert-to-new-aops-fix.patch and perhaps other things which are already queued for 2.6.24 inclusion, so I'll need to ask for an updated patch, please. If the rel_len overflow patch isn't going to make it, then we also need to revert the EXT*_MAX_BLOCK_SIZE change to 65536. It would be possible to allow this to be up to 32768 w/o the rec_len overflow fix however. Ok, thanks, I dropped ext3-support-large-blocksize-up-to-pagesize.patch and ext2-support-large-blocksize-up-to-pagesize.patch. - To unsubscribe from this list: send the line unsubscribe linux-ext4 in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 2/2] ext2: Avoid rec_len overflow with 64KB block size
ext2: Avoid rec_len overflow with 64KB block size From: Jan Kara [EMAIL PROTECTED] With 64KB blocksize, a directory entry can have size 64KB which does not fit into 16 bits we have for entry lenght. So we store 0x instead and convert value when read from / written to disk. Signed-off-by: Jan Kara [EMAIL PROTECTED] Signed-off-by: Mingming Cao [EMAIL PROTECTED] --- fs/ext2/dir.c | 43 +++ include/linux/ext2_fs.h |1 + 2 files changed, 32 insertions(+), 12 deletions(-) diff --git a/fs/ext2/dir.c b/fs/ext2/dir.c index 2bf49d7..1329bdb 100644 --- a/fs/ext2/dir.c +++ b/fs/ext2/dir.c @@ -26,6 +26,24 @@ typedef struct ext2_dir_entry_2 ext2_dirent; +static inline unsigned ext2_rec_len_from_disk(__le16 dlen) +{ + unsigned len = le16_to_cpu(dlen); + + if (len == EXT2_MAX_REC_LEN) + return 1 16; + return len; +} + +static inline __le16 ext2_rec_len_to_disk(unsigned len) +{ + if (len == (1 16)) + return cpu_to_le16(EXT2_MAX_REC_LEN); + else if (len (1 16)) + BUG(); + return cpu_to_le16(len); +} + /* * ext2 uses block-sized chunks. Arguably, sector-sized ones would be * more robust, but we have what we have @@ -95,7 +113,7 @@ static void ext2_check_page(struct page *page) } for (offs = 0; offs = limit - EXT2_DIR_REC_LEN(1); offs += rec_len) { p = (ext2_dirent *)(kaddr + offs); - rec_len = le16_to_cpu(p-rec_len); + rec_len = ext2_rec_len_from_disk(p-rec_len); if (rec_len EXT2_DIR_REC_LEN(1)) goto Eshort; @@ -193,7 +211,8 @@ static inline int ext2_match (int len, const char * const name, */ static inline ext2_dirent *ext2_next_entry(ext2_dirent *p) { - return (ext2_dirent *)((char*)p + le16_to_cpu(p-rec_len)); + return (ext2_dirent *)((char*)p + + ext2_rec_len_from_disk(p-rec_len)); } static inline unsigned @@ -305,7 +324,7 @@ ext2_readdir (struct file * filp, void * dirent, filldir_t filldir) return 0; } } - filp-f_pos += le16_to_cpu(de-rec_len); + filp-f_pos += ext2_rec_len_from_disk(de-rec_len); } ext2_put_page(page); } @@ -413,7 +432,7 @@ void ext2_set_link(struct inode *dir, struct ext2_dir_entry_2 *de, struct page *page, struct inode *inode) { unsigned from = (char *) de - (char *) page_address(page); - unsigned to = from + le16_to_cpu(de-rec_len); + unsigned to = from + ext2_rec_len_from_disk(de-rec_len); int err; lock_page(page); @@ -469,7 +488,7 @@ int ext2_add_link (struct dentry *dentry, struct inode *inode) /* We hit i_size */ name_len = 0; rec_len = chunk_size; - de-rec_len = cpu_to_le16(chunk_size); + de-rec_len = ext2_rec_len_to_disk(chunk_size); de-inode = 0; goto got_it; } @@ -483,7 +502,7 @@ int ext2_add_link (struct dentry *dentry, struct inode *inode) if (ext2_match (namelen, name, de)) goto out_unlock; name_len = EXT2_DIR_REC_LEN(de-name_len); - rec_len = le16_to_cpu(de-rec_len); + rec_len = ext2_rec_len_from_disk(de-rec_len); if (!de-inode rec_len = reclen) goto got_it; if (rec_len = name_len + reclen) @@ -504,8 +523,8 @@ got_it: goto out_unlock; if (de-inode) { ext2_dirent *de1 = (ext2_dirent *) ((char *) de + name_len); - de1-rec_len = cpu_to_le16(rec_len - name_len); - de-rec_len = cpu_to_le16(name_len); + de1-rec_len = ext2_rec_len_to_disk(rec_len - name_len); + de-rec_len = ext2_rec_len_to_disk(name_len); de = de1; } de-name_len = namelen; @@ -536,7 +555,7 @@ int ext2_delete_entry (struct ext2_dir_entry_2 * dir, struct page * page ) struct inode *inode = mapping-host; char *kaddr = page_address(page); unsigned from = ((char*)dir - kaddr) ~(ext2_chunk_size(inode)-1); - unsigned to = ((char*)dir - kaddr) + le16_to_cpu(dir-rec_len); + unsigned to = ((char*)dir - kaddr) + ext2_rec_len_from_disk(dir-rec_len); ext2_dirent * pde = NULL; ext2_dirent * de = (ext2_dirent *) (kaddr + from); int err; @@ -557,7 +576,7 @@ int ext2_delete_entry (struct ext2_dir_entry_2 * dir, struct page * page ) err =
Re: [PATCH 2/2] ext2: Avoid rec_len overflow with 64KB block size
Acked-by: Christoph Lameter [EMAIL PROTECTED] - To unsubscribe from this list: send the line unsubscribe linux-ext4 in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html