* [PATCH 1/4] ext3: Fix ext3_dx_readdir hash collision handling
@ 2008-09-13 15:32 Theodore Ts'o
2008-09-13 15:32 ` [PATCH 2/4] ext3: truncate block allocated on a failed ext3_write_begin Theodore Ts'o
2008-09-17 19:19 ` [PATCH 1/4] ext3: Fix ext3_dx_readdir hash collision handling Andrew Morton
0 siblings, 2 replies; 13+ messages in thread
From: Theodore Ts'o @ 2008-09-13 15:32 UTC (permalink / raw)
To: akpm
Cc: linux-kernel, Theodore Ts'o, Eugene Dashevsky, Mike Snitzer,
linux-ext4
This fixes a bug where readdir() would return a directory entry twice
if there was a hash collision in an hash tree indexed directory.
Signed-off-by: Eugene Dashevsky <eugene@ibrix.com>
Signed-off-by: Mike Snitzer <msnitzer@ibrix.com>
Signed-off-by: "Theodore Ts'o" <tytso@mit.edu>
Cc: linux-ext4@vger.kernel.org
---
fs/ext3/dir.c | 22 ++++++++++++++++------
1 files changed, 16 insertions(+), 6 deletions(-)
diff --git a/fs/ext3/dir.c b/fs/ext3/dir.c
index 2eea96e..42c5391 100644
--- a/fs/ext3/dir.c
+++ b/fs/ext3/dir.c
@@ -410,7 +410,7 @@ static int call_filldir(struct file * filp, void * dirent,
get_dtype(sb, fname->file_type));
if (error) {
filp->f_pos = curr_pos;
- info->extra_fname = fname->next;
+ info->extra_fname = fname;
return error;
}
fname = fname->next;
@@ -449,11 +449,21 @@ static int ext3_dx_readdir(struct file * filp,
* If there are any leftover names on the hash collision
* chain, return them first.
*/
- if (info->extra_fname &&
- call_filldir(filp, dirent, filldir, info->extra_fname))
- goto finished;
^ permalink raw reply related [flat|nested] 13+ messages in thread
* [PATCH 2/4] ext3: truncate block allocated on a failed ext3_write_begin
2008-09-13 15:32 [PATCH 1/4] ext3: Fix ext3_dx_readdir hash collision handling Theodore Ts'o
@ 2008-09-13 15:32 ` Theodore Ts'o
2008-09-13 15:32 ` [PATCH 3/4] ext2: Avoid printk floods in the face of directory corruption Theodore Ts'o
2008-09-17 19:22 ` [PATCH 2/4] ext3: truncate block allocated on a failed ext3_write_begin Andrew Morton
2008-09-17 19:19 ` [PATCH 1/4] ext3: Fix ext3_dx_readdir hash collision handling Andrew Morton
1 sibling, 2 replies; 13+ messages in thread
From: Theodore Ts'o @ 2008-09-13 15:32 UTC (permalink / raw)
To: akpm; +Cc: linux-kernel, Aneesh Kumar K.V, Theodore Ts'o, linux-ext4
From: Aneesh Kumar K.V <aneesh.kumar@linux.vnet.ibm.com>
For blocksize < pagesize we need to remove blocks that got allocated in
block_write_begin() if we fail with ENOSPC for later blocks.
block_write_begin() internally does this if it allocated page
locally. This makes sure we don't have blocks outside inode.i_size
during ENOSPC.
Signed-off-by: Aneesh Kumar K.V <aneesh.kumar@linux.vnet.ibm.com>
Signed-off-by: "Theodore Ts'o" <tytso@mit.edu>
Cc: linux-ext4@vger.kernel.org
---
fs/ext3/inode.c | 7 +++++++
1 files changed, 7 insertions(+), 0 deletions(-)
diff --git a/fs/ext3/inode.c b/fs/ext3/inode.c
index 507d868..bff22b9 100644
--- a/fs/ext3/inode.c
+++ b/fs/ext3/inode.c
@@ -1178,6 +1178,13 @@ write_begin_failed:
ext3_journal_stop(handle);
unlock_page(page);
page_cache_release(page);
+ /*
+ * block_write_begin may have instantiated a few blocks
+ * outside i_size. Trim these off again. Don't need
+ * i_size_read because we hold i_mutex.
+ */
+ if (pos + len > inode->i_size)
+ vmtruncate(inode, inode->i_size);
}
if (ret == -ENOSPC && ext3_should_retry_alloc(inode->i_sb, &retries))
goto retry;
--
1.5.6.1.205.ge2c7.dirty
^ permalink raw reply related [flat|nested] 13+ messages in thread
* [PATCH 3/4] ext2: Avoid printk floods in the face of directory corruption
2008-09-13 15:32 ` [PATCH 2/4] ext3: truncate block allocated on a failed ext3_write_begin Theodore Ts'o
@ 2008-09-13 15:32 ` Theodore Ts'o
2008-09-13 15:32 ` [PATCH 4/4] ext3: " Theodore Ts'o
` (2 more replies)
2008-09-17 19:22 ` [PATCH 2/4] ext3: truncate block allocated on a failed ext3_write_begin Andrew Morton
1 sibling, 3 replies; 13+ messages in thread
From: Theodore Ts'o @ 2008-09-13 15:32 UTC (permalink / raw)
To: akpm; +Cc: linux-kernel, Theodore Ts'o, Eric Sandeen, linux-ext4,
Eugene Teo
Note: some people thinks this represents a security bug, since it
might make the system go away while it is printing a large number of
console messages, especially if a serial console is involved. Hence,
it has been assigned CVE-2008-3528, but it requires that the attacker
either has physical access to your machine to insert a USB disk with a
corrupted filesystem image (at which point why not just hit the power
button), or is otherwise able to convince the system administrator to
mount an arbitrary filesystem image (at which point why not just
include a setuid shell or world-writable hard disk device file or some
such). Me, I think they're just being silly.
Signed-off-by: Eric Sandeen <sandeen@redhat.com>
Signed-off-by: "Theodore Ts'o" <tytso@mit.edu>
Cc: linux-ext4@vger.kernel.org
Cc: Eugene Teo <eugeneteo@kernel.sg>
---
fs/ext2/dir.c | 60 +++++++++++++++++++++++++++++++++-----------------------
1 files changed, 35 insertions(+), 25 deletions(-)
diff --git a/fs/ext2/dir.c b/fs/ext2/dir.c
index a78c6b4..c53c790 100644
--- a/fs/ext2/dir.c
+++ b/fs/ext2/dir.c
@@ -103,7 +103,7 @@ static int ext2_commit_chunk(struct page *page, loff_t pos, unsigned len)
return err;
}
-static void ext2_check_page(struct page *page)
+static void ext2_check_page(struct page *page, int quiet)
{
struct inode *dir = page->mapping->host;
struct super_block *sb = dir->i_sb;
@@ -146,10 +146,10 @@ out:
/* Too bad, we had an error */
Ebadsize:
- ext2_error(sb, "ext2_check_page",
- "size of directory #%lu is not a multiple of chunk size",
- dir->i_ino
- );
+ if (!quiet)
+ ext2_error(sb, __func__,
+ "size of directory #%lu is not a multiple "
+ "of chunk size", dir->i_ino);
goto fail;
Eshort:
error = "rec_len is smaller than minimal";
@@ -166,32 +166,36 @@ Espan:
Einumber:
error = "inode out of bounds";
bad_entry:
- ext2_error (sb, "ext2_check_page", "bad entry in directory #%lu: %s - "
- "offset=%lu, inode=%lu, rec_len=%d, name_len=%d",
- dir->i_ino, error, (page->index<<PAGE_CACHE_SHIFT)+offs,
- (unsigned long) le32_to_cpu(p->inode),
- rec_len, p->name_len);
+ if (!quiet)
+ ext2_error(sb, __func__, "bad entry in directory #%lu: : %s - "
+ "offset=%lu, inode=%lu, rec_len=%d, name_len=%d",
+ dir->i_ino, error, (page->index<<PAGE_CACHE_SHIFT)+offs,
+ (unsigned long) le32_to_cpu(p->inode),
+ rec_len, p->name_len);
goto fail;
Eend:
- p = (ext2_dirent *)(kaddr + offs);
- ext2_error (sb, "ext2_check_page",
- "entry in directory #%lu spans the page boundary"
- "offset=%lu, inode=%lu",
- dir->i_ino, (page->index<<PAGE_CACHE_SHIFT)+offs,
- (unsigned long) le32_to_cpu(p->inode));
+ if (!quiet) {
+ p = (ext2_dirent *)(kaddr + offs);
+ ext2_error (sb, "ext2_check_page",
+ "entry in directory #%lu spans the page boundary"
+ "offset=%lu, inode=%lu",
+ dir->i_ino, (page->index<<PAGE_CACHE_SHIFT)+offs,
+ (unsigned long) le32_to_cpu(p->inode));
+ }
fail:
SetPageChecked(page);
SetPageError(page);
}
-static struct page * ext2_get_page(struct inode *dir, unsigned long n)
+static struct page * ext2_get_page(struct inode *dir, unsigned long n,
+ int quiet)
{
struct address_space *mapping = dir->i_mapping;
struct page *page = read_mapping_page(mapping, n, NULL);
if (!IS_ERR(page)) {
kmap(page);
if (!PageChecked(page))
- ext2_check_page(page);
+ ext2_check_page(page, quiet);
if (PageError(page))
goto fail;
}
@@ -292,7 +296,7 @@ ext2_readdir (struct file * filp, void * dirent, filldir_t filldir)
for ( ; n < npages; n++, offset = 0) {
char *kaddr, *limit;
ext2_dirent *de;
- struct page *page = ext2_get_page(inode, n);
+ struct page *page = ext2_get_page(inode, n, 0);
if (IS_ERR(page)) {
ext2_error(sb, __func__,
@@ -361,6 +365,7 @@ struct ext2_dir_entry_2 * ext2_find_entry (struct inode * dir,
struct page *page = NULL;
struct ext2_inode_info *ei = EXT2_I(dir);
ext2_dirent * de;
+ int dir_has_error = 0;
if (npages == 0)
goto out;
@@ -374,7 +379,7 @@ struct ext2_dir_entry_2 * ext2_find_entry (struct inode * dir,
n = start;
do {
char *kaddr;
- page = ext2_get_page(dir, n);
+ page = ext2_get_page(dir, n, dir_has_error);
if (!IS_ERR(page)) {
kaddr = page_address(page);
de = (ext2_dirent *) kaddr;
@@ -391,7 +396,9 @@ struct ext2_dir_entry_2 * ext2_find_entry (struct inode * dir,
de = ext2_next_entry(de);
}
ext2_put_page(page);
- }
+ } else
+ dir_has_error = 1;
+
if (++n >= npages)
n = 0;
/* next page is past the blocks we've got */
@@ -414,7 +421,7 @@ found:
struct ext2_dir_entry_2 * ext2_dotdot (struct inode *dir, struct page **p)
{
- struct page *page = ext2_get_page(dir, 0);
+ struct page *page = ext2_get_page(dir, 0, 0);
ext2_dirent *de = NULL;
if (!IS_ERR(page)) {
@@ -487,7 +494,7 @@ int ext2_add_link (struct dentry *dentry, struct inode *inode)
for (n = 0; n <= npages; n++) {
char *dir_end;
- page = ext2_get_page(dir, n);
+ page = ext2_get_page(dir, n, 0);
err = PTR_ERR(page);
if (IS_ERR(page))
goto out;
@@ -655,14 +662,17 @@ int ext2_empty_dir (struct inode * inode)
{
struct page *page = NULL;
unsigned long i, npages = dir_pages(inode);
+ int dir_has_error = 0;
for (i = 0; i < npages; i++) {
char *kaddr;
ext2_dirent * de;
- page = ext2_get_page(inode, i);
+ page = ext2_get_page(inode, i, dir_has_error);
- if (IS_ERR(page))
+ if (IS_ERR(page)) {
+ dir_has_error = 1;
continue;
+ }
kaddr = page_address(page);
de = (ext2_dirent *)kaddr;
--
1.5.6.1.205.ge2c7.dirty
^ permalink raw reply related [flat|nested] 13+ messages in thread
* [PATCH 4/4] ext3: Avoid printk floods in the face of directory corruption
2008-09-13 15:32 ` [PATCH 3/4] ext2: Avoid printk floods in the face of directory corruption Theodore Ts'o
@ 2008-09-13 15:32 ` Theodore Ts'o
2008-09-18 0:57 ` Eugene Teo
2008-09-17 19:25 ` [PATCH 3/4] ext2: " Andrew Morton
2008-09-18 9:46 ` Eugene Teo
2 siblings, 1 reply; 13+ messages in thread
From: Theodore Ts'o @ 2008-09-13 15:32 UTC (permalink / raw)
To: akpm; +Cc: linux-kernel, Theodore Ts'o, Eric Sandeen, linux-ext4,
Eugene Teo
Note: some people thinks this represents a security bug, since it
might make the system go away while it is printing a large number of
console messages, especially if a serial console is involved. Hence,
it has been assigned CVE-2008-3528, but it requires that the attacker
either has physical access to your machine to insert a USB disk with a
corrupted filesystem image (at which point why not just hit the power
button), or is otherwise able to convince the system administrator to
mount an arbitrary filesystem image (at which point why not just
include a setuid shell or world-writable hard disk device file or some
such). Me, I think they're just being silly.
Signed-off-by: Eric Sandeen <sandeen@redhat.com>
Signed-off-by: "Theodore Ts'o" <tytso@mit.edu>
Cc: linux-ext4@vger.kernel.org
Cc: Eugene Teo <eugeneteo@kernel.sg>
---
fs/ext3/dir.c | 10 +++++++---
1 files changed, 7 insertions(+), 3 deletions(-)
diff --git a/fs/ext3/dir.c b/fs/ext3/dir.c
index 42c5391..283938a 100644
--- a/fs/ext3/dir.c
+++ b/fs/ext3/dir.c
@@ -102,6 +102,7 @@ static int ext3_readdir(struct file * filp,
int err;
struct inode *inode = filp->f_path.dentry->d_inode;
int ret = 0;
+ int dir_has_error = 0;
sb = inode->i_sb;
@@ -148,9 +149,12 @@ static int ext3_readdir(struct file * filp,
* of recovering data when there's a bad sector
*/
if (!bh) {
- ext3_error (sb, "ext3_readdir",
- "directory #%lu contains a hole at offset %lu",
- inode->i_ino, (unsigned long)filp->f_pos);
+ if (!dir_has_error) {
+ ext3_error(sb, __func__, "directory #%lu "
+ "contains a hole at offset %lld",
+ inode->i_ino, filp->f_pos);
+ dir_has_error = 1;
+ }
/* corrupt size? Maybe no more blocks to read */
if (filp->f_pos > inode->i_blocks << 9)
break;
--
1.5.6.1.205.ge2c7.dirty
^ permalink raw reply related [flat|nested] 13+ messages in thread
* Re: [PATCH 1/4] ext3: Fix ext3_dx_readdir hash collision handling
2008-09-13 15:32 [PATCH 1/4] ext3: Fix ext3_dx_readdir hash collision handling Theodore Ts'o
2008-09-13 15:32 ` [PATCH 2/4] ext3: truncate block allocated on a failed ext3_write_begin Theodore Ts'o
@ 2008-09-17 19:19 ` Andrew Morton
2008-10-01 22:37 ` Theodore Tso
1 sibling, 1 reply; 13+ messages in thread
From: Andrew Morton @ 2008-09-17 19:19 UTC (permalink / raw)
To: Theodore Ts'o; +Cc: linux-kernel, tytso, eugene, msnitzer, linux-ext4
On Sat, 13 Sep 2008 11:32:48 -0400
"Theodore Ts'o" <tytso@MIT.EDU> wrote:
> This fixes a bug where readdir() would return a directory entry twice
> if there was a hash collision in an hash tree indexed directory.
That sounds like a serious problem, but given the amount of time it
took to turn up, I guess it's pretty rare.
What are your thoughts regarding a 2.6.27 merge for this? 2.6.26.x?
2.6.25.x? ...
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH 2/4] ext3: truncate block allocated on a failed ext3_write_begin
2008-09-13 15:32 ` [PATCH 2/4] ext3: truncate block allocated on a failed ext3_write_begin Theodore Ts'o
2008-09-13 15:32 ` [PATCH 3/4] ext2: Avoid printk floods in the face of directory corruption Theodore Ts'o
@ 2008-09-17 19:22 ` Andrew Morton
2008-09-18 7:03 ` Aneesh Kumar K.V
1 sibling, 1 reply; 13+ messages in thread
From: Andrew Morton @ 2008-09-17 19:22 UTC (permalink / raw)
To: Theodore Ts'o
Cc: linux-kernel, aneesh.kumar, tytso, linux-ext4, Nick Piggin
On Sat, 13 Sep 2008 11:32:49 -0400
"Theodore Ts'o" <tytso@MIT.EDU> wrote:
> From: Aneesh Kumar K.V <aneesh.kumar@linux.vnet.ibm.com>
>
> For blocksize < pagesize we need to remove blocks that got allocated in
> block_write_begin() if we fail with ENOSPC for later blocks.
> block_write_begin() internally does this if it allocated page
> locally. This makes sure we don't have blocks outside inode.i_size
> during ENOSPC.
>
> Signed-off-by: Aneesh Kumar K.V <aneesh.kumar@linux.vnet.ibm.com>
> Signed-off-by: "Theodore Ts'o" <tytso@mit.edu>
> Cc: linux-ext4@vger.kernel.org
> ---
> fs/ext3/inode.c | 7 +++++++
> 1 files changed, 7 insertions(+), 0 deletions(-)
>
> diff --git a/fs/ext3/inode.c b/fs/ext3/inode.c
> index 507d868..bff22b9 100644
> --- a/fs/ext3/inode.c
> +++ b/fs/ext3/inode.c
> @@ -1178,6 +1178,13 @@ write_begin_failed:
> ext3_journal_stop(handle);
> unlock_page(page);
> page_cache_release(page);
> + /*
> + * block_write_begin may have instantiated a few blocks
> + * outside i_size. Trim these off again. Don't need
> + * i_size_read because we hold i_mutex.
> + */
> + if (pos + len > inode->i_size)
> + vmtruncate(inode, inode->i_size);
> }
> if (ret == -ENOSPC && ext3_should_retry_alloc(inode->i_sb, &retries))
> goto retry;
Well we used to do this trimming in core VFS, but Nick broke it. We
still do it if the fs doesn't implement ->write_begin().
Should we do this trimming in pagecache_write_begin() in both cases?
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH 3/4] ext2: Avoid printk floods in the face of directory corruption
2008-09-13 15:32 ` [PATCH 3/4] ext2: Avoid printk floods in the face of directory corruption Theodore Ts'o
2008-09-13 15:32 ` [PATCH 4/4] ext3: " Theodore Ts'o
@ 2008-09-17 19:25 ` Andrew Morton
2008-09-17 19:30 ` Eric Sandeen
2008-09-18 9:46 ` Eugene Teo
2 siblings, 1 reply; 13+ messages in thread
From: Andrew Morton @ 2008-09-17 19:25 UTC (permalink / raw)
To: Theodore Ts'o; +Cc: linux-kernel, tytso, sandeen, linux-ext4, eugeneteo
On Sat, 13 Sep 2008 11:32:50 -0400
"Theodore Ts'o" <tytso@MIT.EDU> wrote:
> From: "Theodore Ts'o" <tytso@MIT.EDU>
> To: akpm@linux-foundation.org
> Cc: linux-kernel@vger.kernel.org, "Theodore Ts'o" <tytso@MIT.EDU>, Eric Sandeen <sandeen@redhat.com>, linux-ext4@vger.kernel.org, Eugene Teo <eugeneteo@kernel.sg>
> Subject: [PATCH 3/4] ext2: Avoid printk floods in the face of directory corruption
> Date: Sat, 13 Sep 2008 11:32:50 -0400
> X-Mailer: git-send-email 1.5.6.1.205.ge2c7.dirty
>
> Note: some people thinks this represents a security bug, since it
> might make the system go away while it is printing a large number of
> console messages, especially if a serial console is involved. Hence,
> it has been assigned CVE-2008-3528, but it requires that the attacker
> either has physical access to your machine to insert a USB disk with a
> corrupted filesystem image (at which point why not just hit the power
> button), or is otherwise able to convince the system administrator to
> mount an arbitrary filesystem image (at which point why not just
> include a setuid shell or world-writable hard disk device file or some
> such). Me, I think they're just being silly.
>
> Signed-off-by: Eric Sandeen <sandeen@redhat.com>
> Signed-off-by: "Theodore Ts'o" <tytso@mit.edu>
> Cc: linux-ext4@vger.kernel.org
> Cc: Eugene Teo <eugeneteo@kernel.sg>
This patch was purportedly authored by yourself, but I'm going to
assume that it was authored by Eric.
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH 3/4] ext2: Avoid printk floods in the face of directory corruption
2008-09-17 19:25 ` [PATCH 3/4] ext2: " Andrew Morton
@ 2008-09-17 19:30 ` Eric Sandeen
0 siblings, 0 replies; 13+ messages in thread
From: Eric Sandeen @ 2008-09-17 19:30 UTC (permalink / raw)
To: Andrew Morton; +Cc: Theodore Ts'o, linux-kernel, linux-ext4, eugeneteo
Andrew Morton wrote:
> On Sat, 13 Sep 2008 11:32:50 -0400
> "Theodore Ts'o" <tytso@MIT.EDU> wrote:
>
>> From: "Theodore Ts'o" <tytso@MIT.EDU>
>> To: akpm@linux-foundation.org
>> Cc: linux-kernel@vger.kernel.org, "Theodore Ts'o" <tytso@MIT.EDU>, Eric Sandeen <sandeen@redhat.com>, linux-ext4@vger.kernel.org, Eugene Teo <eugeneteo@kernel.sg>
>> Subject: [PATCH 3/4] ext2: Avoid printk floods in the face of directory corruption
>> Date: Sat, 13 Sep 2008 11:32:50 -0400
>> X-Mailer: git-send-email 1.5.6.1.205.ge2c7.dirty
>>
>> Note: some people thinks this represents a security bug, since it
>> might make the system go away while it is printing a large number of
>> console messages, especially if a serial console is involved. Hence,
>> it has been assigned CVE-2008-3528, but it requires that the attacker
>> either has physical access to your machine to insert a USB disk with a
>> corrupted filesystem image (at which point why not just hit the power
>> button), or is otherwise able to convince the system administrator to
>> mount an arbitrary filesystem image (at which point why not just
>> include a setuid shell or world-writable hard disk device file or some
>> such). Me, I think they're just being silly.
>>
>> Signed-off-by: Eric Sandeen <sandeen@redhat.com>
>> Signed-off-by: "Theodore Ts'o" <tytso@mit.edu>
>> Cc: linux-ext4@vger.kernel.org
>> Cc: Eugene Teo <eugeneteo@kernel.sg>
>
> This patch was purportedly authored by yourself, but I'm going to
> assume that it was authored by Eric.
It was, after some discussion w/ Ted & Andreas.
Also just FWIW I'm also in the "as a security issue this is a bit silly"
camp ;)
Thanks,
-Eric
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH 4/4] ext3: Avoid printk floods in the face of directory corruption
2008-09-13 15:32 ` [PATCH 4/4] ext3: " Theodore Ts'o
@ 2008-09-18 0:57 ` Eugene Teo
0 siblings, 0 replies; 13+ messages in thread
From: Eugene Teo @ 2008-09-18 0:57 UTC (permalink / raw)
To: Theodore Ts'o; +Cc: akpm, linux-kernel, Eric Sandeen, linux-ext4
On Sat, Sep 13, 2008 at 11:32 PM, Theodore Ts'o <tytso@mit.edu> wrote:
> Note: some people thinks this represents a security bug, since it
> might make the system go away while it is printing a large number of
> console messages, especially if a serial console is involved. Hence,
> it has been assigned CVE-2008-3528, but it requires that the attacker
> either has physical access to your machine to insert a USB disk with a
> corrupted filesystem image (at which point why not just hit the power
> button), or is otherwise able to convince the system administrator to
> mount an arbitrary filesystem image (at which point why not just
> include a setuid shell or world-writable hard disk device file or some
> such). Me, I think they're just being silly.
The description should explain what the problem is. And the last
sentence is a little ambiguous.
This is a user-triggerable DoS. The administrator who mounted the
filesystem image or partition might not know that the dir->i_size and
dir->i_blocks are corrupted. A remote user just need to perform either
a read or write operation to the mounted image or partition, and this
could trigger the problem, resulting in a denial of service.
Take note that another problem the test image shows is that, the
ext2/3 (and possibly ext4) filesystem does not honour the read-only
mode when the revision level is too high. That is, when
le32_to_cpu(es->s_rev_level) > EXT3_MAX_SUPP_REV. Eric replied me in a
private email that this is a different, and unrelated bug that will be
fixed.
Thanks, Eugene
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH 2/4] ext3: truncate block allocated on a failed ext3_write_begin
2008-09-17 19:22 ` [PATCH 2/4] ext3: truncate block allocated on a failed ext3_write_begin Andrew Morton
@ 2008-09-18 7:03 ` Aneesh Kumar K.V
0 siblings, 0 replies; 13+ messages in thread
From: Aneesh Kumar K.V @ 2008-09-18 7:03 UTC (permalink / raw)
To: Andrew Morton; +Cc: Theodore Ts'o, linux-kernel, linux-ext4, Nick Piggin
On Wed, Sep 17, 2008 at 12:22:54PM -0700, Andrew Morton wrote:
> On Sat, 13 Sep 2008 11:32:49 -0400
> "Theodore Ts'o" <tytso@MIT.EDU> wrote:
>
> > From: Aneesh Kumar K.V <aneesh.kumar@linux.vnet.ibm.com>
> >
> > For blocksize < pagesize we need to remove blocks that got allocated in
> > block_write_begin() if we fail with ENOSPC for later blocks.
> > block_write_begin() internally does this if it allocated page
> > locally. This makes sure we don't have blocks outside inode.i_size
> > during ENOSPC.
> >
> > Signed-off-by: Aneesh Kumar K.V <aneesh.kumar@linux.vnet.ibm.com>
> > Signed-off-by: "Theodore Ts'o" <tytso@mit.edu>
> > Cc: linux-ext4@vger.kernel.org
> > ---
> > fs/ext3/inode.c | 7 +++++++
> > 1 files changed, 7 insertions(+), 0 deletions(-)
> >
> > diff --git a/fs/ext3/inode.c b/fs/ext3/inode.c
> > index 507d868..bff22b9 100644
> > --- a/fs/ext3/inode.c
> > +++ b/fs/ext3/inode.c
> > @@ -1178,6 +1178,13 @@ write_begin_failed:
> > ext3_journal_stop(handle);
> > unlock_page(page);
> > page_cache_release(page);
> > + /*
> > + * block_write_begin may have instantiated a few blocks
> > + * outside i_size. Trim these off again. Don't need
> > + * i_size_read because we hold i_mutex.
> > + */
> > + if (pos + len > inode->i_size)
> > + vmtruncate(inode, inode->i_size);
> > }
> > if (ret == -ENOSPC && ext3_should_retry_alloc(inode->i_sb, &retries))
> > goto retry;
>
> Well we used to do this trimming in core VFS, but Nick broke it. We
> still do it if the fs doesn't implement ->write_begin().
We still do it in block_write_begin if the pages are allocated by
block_write_begin.
>
> Should we do this trimming in pagecache_write_begin() in both cases?
pagecache_write_begin is not used in the write_begin call path for
ext3/ext4.
generic_file_buffered_write
generic_perform_write
ext3_write_begin
block_write_begin
-aneesh
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH 3/4] ext2: Avoid printk floods in the face of directory corruption
2008-09-13 15:32 ` [PATCH 3/4] ext2: Avoid printk floods in the face of directory corruption Theodore Ts'o
2008-09-13 15:32 ` [PATCH 4/4] ext3: " Theodore Ts'o
2008-09-17 19:25 ` [PATCH 3/4] ext2: " Andrew Morton
@ 2008-09-18 9:46 ` Eugene Teo
2 siblings, 0 replies; 13+ messages in thread
From: Eugene Teo @ 2008-09-18 9:46 UTC (permalink / raw)
To: Theodore Ts'o; +Cc: akpm, linux-kernel, Eric Sandeen, linux-ext4
On Sat, Sep 13, 2008 at 11:32 PM, Theodore Ts'o <tytso@mit.edu> wrote:
[...]
> Eend:
> - p = (ext2_dirent *)(kaddr + offs);
> - ext2_error (sb, "ext2_check_page",
> - "entry in directory #%lu spans the page boundary"
> - "offset=%lu, inode=%lu",
> - dir->i_ino, (page->index<<PAGE_CACHE_SHIFT)+offs,
> - (unsigned long) le32_to_cpu(p->inode));
> + if (!quiet) {
> + p = (ext2_dirent *)(kaddr + offs);
> + ext2_error (sb, "ext2_check_page",
^^^^^^^^^^^^^^^^^^
> + "entry in directory #%lu spans the page boundary"
> + "offset=%lu, inode=%lu",
> + dir->i_ino, (page->index<<PAGE_CACHE_SHIFT)+offs,
> + (unsigned long) le32_to_cpu(p->inode));
Minor issue. Since you are already changing "ext2_check_page" to
__func__, you might as well change it here too.
Thanks, Eugene
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH 1/4] ext3: Fix ext3_dx_readdir hash collision handling
2008-09-17 19:19 ` [PATCH 1/4] ext3: Fix ext3_dx_readdir hash collision handling Andrew Morton
@ 2008-10-01 22:37 ` Theodore Tso
2008-10-01 23:33 ` Andrew Morton
0 siblings, 1 reply; 13+ messages in thread
From: Theodore Tso @ 2008-10-01 22:37 UTC (permalink / raw)
To: Andrew Morton; +Cc: linux-kernel, eugene, msnitzer, linux-ext4
On Wed, Sep 17, 2008 at 12:19:43PM -0700, Andrew Morton wrote:
> On Sat, 13 Sep 2008 11:32:48 -0400
> "Theodore Ts'o" <tytso@MIT.EDU> wrote:
>
> > This fixes a bug where readdir() would return a directory entry twice
> > if there was a hash collision in an hash tree indexed directory.
>
> That sounds like a serious problem, but given the amount of time it
> took to turn up, I guess it's pretty rare.
>
> What are your thoughts regarding a 2.6.27 merge for this? 2.6.26.x?
> 2.6.25.x? ...
>
It's not a regression, so per Linus's request that at this point we're
too late for anything other than regression fixes, I had assumed that
it should be pushed for 2.6.28, and then go into the various stable
trees. It's true that it took quite a while for people to notice,
probably because many programs won't notice if readdir() returns a
directory entry twice.
BTW, this hasn't hit -mm yet, and I've got a number of ext3 patches
that don't appear to have hit -mm, including one that was authored by
Linus. Should I create a git tree or a quilt series if that would
make things easier for you?
- Ted
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH 1/4] ext3: Fix ext3_dx_readdir hash collision handling
2008-10-01 22:37 ` Theodore Tso
@ 2008-10-01 23:33 ` Andrew Morton
0 siblings, 0 replies; 13+ messages in thread
From: Andrew Morton @ 2008-10-01 23:33 UTC (permalink / raw)
To: Theodore Tso; +Cc: linux-kernel, eugene, msnitzer, linux-ext4
On Wed, 1 Oct 2008 18:37:55 -0400
Theodore Tso <tytso@mit.edu> wrote:
> On Wed, Sep 17, 2008 at 12:19:43PM -0700, Andrew Morton wrote:
> > On Sat, 13 Sep 2008 11:32:48 -0400
> > "Theodore Ts'o" <tytso@MIT.EDU> wrote:
> >
> > > This fixes a bug where readdir() would return a directory entry twice
> > > if there was a hash collision in an hash tree indexed directory.
> >
> > That sounds like a serious problem, but given the amount of time it
> > took to turn up, I guess it's pretty rare.
> >
> > What are your thoughts regarding a 2.6.27 merge for this? 2.6.26.x?
> > 2.6.25.x? ...
> >
>
> It's not a regression, so per Linus's request that at this point we're
> too late for anything other than regression fixes, I had assumed that
> it should be pushed for 2.6.28, and then go into the various stable
> trees. It's true that it took quite a while for people to notice,
> probably because many programs won't notice if readdir() returns a
> directory entry twice.
OK.
> BTW, this hasn't hit -mm yet, and I've got a number of ext3 patches
> that don't appear to have hit -mm, including one that was authored by
> Linus. Should I create a git tree or a quilt series if that would
> make things easier for you?
>
I currently have:
jbd-abort-when-failed-to-log-metadata-buffers.patch
#jbd-fix-error-handling-for-checkpoint-io.patch: double-check this
jbd-fix-error-handling-for-checkpoint-io.patch
ext3-add-checks-for-errors-from-jbd.patch
jbd-dont-dirty-original-metadata-buffer-on-abort.patch
ext3-dont-try-to-resize-if-there-are-no-reserved-gdt-blocks-left.patch
ext3-fix-ext3-block-reservation-early-enospc-issue.patch
jbd-test-bh_write_eio-to-detect-errors-on-metadata-buffers.patch
ext3-add-an-option-to-control-error-handling-on-file-data.patch
jbd-ordered-data-integrity-fix.patch
ext3-fix-ext3_dx_readdir-hash-collision-handling.patch
ext3-fix-ext3_dx_readdir-hash-collision-handling-checkpatch-fixes.patch
ext3-truncate-block-allocated-on-a-failed-ext3_write_begin.patch
ext3-avoid-printk-floods-in-the-face-of-directory-corruption.patch
#jbd-abort-instead-of-waiting-for-nonexistent-transactions.patch: sct probs
jbd-abort-instead-of-waiting-for-nonexistent-transactions.patch
and I don't see anything from yourself in the backlog?
There are some ext3 patches from other people I need to go through -
I've been offlineish for nearly a week and have spent a fun day staring
in horror at the mess which people are shovelling in Stephen's
direction for linux-next.
^ permalink raw reply [flat|nested] 13+ messages in thread
end of thread, other threads:[~2008-10-01 23:33 UTC | newest]
Thread overview: 13+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2008-09-13 15:32 [PATCH 1/4] ext3: Fix ext3_dx_readdir hash collision handling Theodore Ts'o
2008-09-13 15:32 ` [PATCH 2/4] ext3: truncate block allocated on a failed ext3_write_begin Theodore Ts'o
2008-09-13 15:32 ` [PATCH 3/4] ext2: Avoid printk floods in the face of directory corruption Theodore Ts'o
2008-09-13 15:32 ` [PATCH 4/4] ext3: " Theodore Ts'o
2008-09-18 0:57 ` Eugene Teo
2008-09-17 19:25 ` [PATCH 3/4] ext2: " Andrew Morton
2008-09-17 19:30 ` Eric Sandeen
2008-09-18 9:46 ` Eugene Teo
2008-09-17 19:22 ` [PATCH 2/4] ext3: truncate block allocated on a failed ext3_write_begin Andrew Morton
2008-09-18 7:03 ` Aneesh Kumar K.V
2008-09-17 19:19 ` [PATCH 1/4] ext3: Fix ext3_dx_readdir hash collision handling Andrew Morton
2008-10-01 22:37 ` Theodore Tso
2008-10-01 23:33 ` Andrew Morton
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).