Discussion:
Patch "ext4: propagate errors up to ext4_find_entry()'s callers" has been added to the 3.16-stable tree
g***@linuxfoundation.org
2014-09-03 21:36:11 UTC
Permalink
This is a note to let you know that I've just added the patch titled

ext4: propagate errors up to ext4_find_entry()'s callers

to the 3.16-stable tree which can be found at:
http://www.kernel.org/git/?p=linux/kernel/git/stable/stable-queue.git;a=summary

The filename of the patch is:
ext4-propagate-errors-up-to-ext4_find_entry-s-callers.patch
and it can be found in the queue-3.16 subdirectory.

If you, or anyone else, feels it should not be added to the stable tree,
From 36de928641ee48b2078d3fe9514242aaa2f92013 Mon Sep 17 00:00:00 2001
From: Theodore Ts'o <***@mit.edu>
Date: Sat, 23 Aug 2014 17:47:19 -0400
Subject: ext4: propagate errors up to ext4_find_entry()'s callers

From: Theodore Ts'o <***@mit.edu>

commit 36de928641ee48b2078d3fe9514242aaa2f92013 upstream.

If we run into some kind of error, such as ENOMEM, while calling
ext4_getblk() or ext4_dx_find_entry(), we need to make sure this error
gets propagated up to ext4_find_entry() and then to its callers. This
way, transient errors such as ENOMEM can get propagated to the VFS.
This is important so that the system calls return the appropriate
error, and also so that in the case of ext4_lookup(), we return an
error instead of a NULL inode, since that will result in a negative
dentry cache entry that will stick around long past the OOM condition
which caused a transient ENOMEM error.

Google-Bug-Id: #17142205

Signed-off-by: Theodore Ts'o <***@mit.edu>
Signed-off-by: Greg Kroah-Hartman <***@linuxfoundation.org>

---
fs/ext4/ext4.h | 2 +-
fs/ext4/namei.c | 35 +++++++++++++++++++++++++++++++++--
2 files changed, 34 insertions(+), 3 deletions(-)

--- a/fs/ext4/ext4.h
+++ b/fs/ext4/ext4.h
@@ -1826,7 +1826,7 @@ ext4_group_first_block_no(struct super_b
/*
* Special error return code only used by dx_probe() and its callers.
*/
-#define ERR_BAD_DX_DIR -75000
+#define ERR_BAD_DX_DIR (-(MAX_ERRNO - 1))

/*
* Timeout and state flag for lazy initialization inode thread.
--- a/fs/ext4/namei.c
+++ b/fs/ext4/namei.c
@@ -1227,7 +1227,7 @@ static struct buffer_head * ext4_find_en
buffer */
int num = 0;
ext4_lblk_t nblocks;
- int i, err;
+ int i, err = 0;
int namelen;

*res_dir = NULL;
@@ -1264,7 +1264,11 @@ static struct buffer_head * ext4_find_en
* return. Otherwise, fall back to doing a search the
* old fashioned way.
*/
- if (bh || (err != ERR_BAD_DX_DIR))
+ if (err == -ENOENT)
+ return NULL;
+ if (err && err != ERR_BAD_DX_DIR)
+ return ERR_PTR(err);
+ if (bh)
return bh;
dxtrace(printk(KERN_DEBUG "ext4_find_entry: dx failed, "
"falling back\n"));
@@ -1295,6 +1299,11 @@ restart:
}
num++;
bh = ext4_getblk(NULL, dir, b++, 0, &err);
+ if (unlikely(err)) {
+ if (ra_max == 0)
+ return ERR_PTR(err);
+ break;
+ }
bh_use[ra_max] = bh;
if (bh)
ll_rw_block(READ | REQ_META | REQ_PRIO,
@@ -1417,6 +1426,8 @@ static struct dentry *ext4_lookup(struct
return ERR_PTR(-ENAMETOOLONG);

bh = ext4_find_entry(dir, &dentry->d_name, &de, NULL);
+ if (IS_ERR(bh))
+ return (struct dentry *) bh;
inode = NULL;
if (bh) {
__u32 ino = le32_to_cpu(de->inode);
@@ -1450,6 +1461,8 @@ struct dentry *ext4_get_parent(struct de
struct buffer_head *bh;

bh = ext4_find_entry(child->d_inode, &dotdot, &de, NULL);
+ if (IS_ERR(bh))
+ return (struct dentry *) bh;
if (!bh)
return ERR_PTR(-ENOENT);
ino = le32_to_cpu(de->inode);
@@ -2727,6 +2740,8 @@ static int ext4_rmdir(struct inode *dir,

retval = -ENOENT;
bh = ext4_find_entry(dir, &dentry->d_name, &de, NULL);
+ if (IS_ERR(bh))
+ return PTR_ERR(bh);
if (!bh)
goto end_rmdir;

@@ -2794,6 +2809,8 @@ static int ext4_unlink(struct inode *dir

retval = -ENOENT;
bh = ext4_find_entry(dir, &dentry->d_name, &de, NULL);
+ if (IS_ERR(bh))
+ return PTR_ERR(bh);
if (!bh)
goto end_unlink;

@@ -3121,6 +3138,8 @@ static int ext4_find_delete_entry(handle
struct ext4_dir_entry_2 *de;

bh = ext4_find_entry(dir, d_name, &de, NULL);
+ if (IS_ERR(bh))
+ return PTR_ERR(bh);
if (bh) {
retval = ext4_delete_entry(handle, dir, de, bh);
brelse(bh);
@@ -3202,6 +3221,8 @@ static int ext4_rename(struct inode *old
dquot_initialize(new.inode);

old.bh = ext4_find_entry(old.dir, &old.dentry->d_name, &old.de, NULL);
+ if (IS_ERR(old.bh))
+ return PTR_ERR(old.bh);
/*
* Check for inode number is _not_ due to possible IO errors.
* We might rmdir the source, keep it as pwd of some process
@@ -3214,6 +3235,10 @@ static int ext4_rename(struct inode *old

new.bh = ext4_find_entry(new.dir, &new.dentry->d_name,
&new.de, &new.inlined);
+ if (IS_ERR(new.bh)) {
+ retval = PTR_ERR(new.bh);
+ goto end_rename;
+ }
if (new.bh) {
if (!new.inode) {
brelse(new.bh);
@@ -3330,6 +3355,8 @@ static int ext4_cross_rename(struct inod

old.bh = ext4_find_entry(old.dir, &old.dentry->d_name,
&old.de, &old.inlined);
+ if (IS_ERR(old.bh))
+ return PTR_ERR(old.bh);
/*
* Check for inode number is _not_ due to possible IO errors.
* We might rmdir the source, keep it as pwd of some process
@@ -3342,6 +3369,10 @@ static int ext4_cross_rename(struct inod

new.bh = ext4_find_entry(new.dir, &new.dentry->d_name,
&new.de, &new.inlined);
+ if (IS_ERR(new.bh)) {
+ retval = PTR_ERR(new.bh);
+ goto end_rename;
+ }

/* RENAME_EXCHANGE case: old *and* new must both exist */
if (!new.bh || le32_to_cpu(new.de->inode) != new.inode->i_ino)


Patches currently in stable-queue which might be from ***@mit.edu are

queue-3.16/ext4-move-i_size-i_disksize-update-routines-to-helper-function.patch
queue-3.16/ext4-fix-punch-hole-on-files-with-indirect-mapping.patch
queue-3.16/ext4-fix-ext4_discard_allocated_blocks-if-we-can-t-allocate-the-pa-struct.patch
queue-3.16/ext4-fix-bug_on-in-mb_free_blocks.patch
queue-3.16/ext4-propagate-errors-up-to-ext4_find_entry-s-callers.patch
Theodore Ts'o
2014-09-03 22:06:05 UTC
Permalink
Post by g***@linuxfoundation.org
This is a note to let you know that I've just added the patch titled
ext4: propagate errors up to ext4_find_entry()'s callers
I hate to do this again, but could you hold up this patch until the
next stable release?

The following hasn't been pushed out to Linus yet, but it fixes a bug
which was overlooked in the review. (Ironically, I had found another
instance of the same mistake elsewhere in an earlier version of this
commit patch, and asked Dan Carpenter to code up a smatch patch to
scan for this mistake, since it's so easy to make/miss --- and then
promptly proved this by missing the other two instances of the same
mistake in fs/namei.c in that patch. Sigh...)

http://git.kernel.org/cgit/linux/kernel/git/tytso/ext4.git/commit/?h=for_linus_urgent

ext4: avoid trying to kfree an ERR_PTR pointer

Thanks to Dan Carpenter for extending smatch to find bugs like this.
(This was found using a development version of smatch.)

Fixes: 36de928641ee48b2078d3fe9514242aaa2f92013
Reported-by: Dan Carpenter <***@oracle.com
Signed-off-by: Theodore Ts'o <***@mit.edu>
Cc: ***@vger.kernel.org

The incidence rate of the bug fixed by 36de928641e is pretty rare,
especially in the upstream kernel specially in the upstream kernel, so
it shouldn't be too bad to just hold back this fix for a week or so.

Thanks, and apologies for the inconvenience.

- Ted
Greg KH
2014-09-03 23:40:27 UTC
Permalink
Post by Theodore Ts'o
Post by g***@linuxfoundation.org
This is a note to let you know that I've just added the patch titled
ext4: propagate errors up to ext4_find_entry()'s callers
I hate to do this again, but could you hold up this patch until the
next stable release?
Ok, now dropped, please resend it to ***@vger.kernel.org when you
want it to go in.

thanks,

greg k-h
Theodore Ts'o
2014-09-09 17:00:47 UTC
Permalink
Post by Greg KH
Post by Theodore Ts'o
Post by g***@linuxfoundation.org
ext4: propagate errors up to ext4_find_entry()'s callers
I hate to do this again, but could you hold up this patch until the
next stable release?
want it to go in.
Hi Greg and other stable kernel maintainers,

Please cherry-pick the following commits from mainline into your
stable kernels:

36de928 ext4: propagate errors up to ext4_find_entry()'s callers
a9cfcd6 ext4: avoid trying to kfree an ERR_PTR pointer

There was a bug accidentally introduced 36de928 which is fixed in
a9cfcd6, hence my request to Greg that he hold up the first commit.

Many thanks!!

- Ted
Greg KH
2014-10-03 21:15:17 UTC
Permalink
Post by Theodore Ts'o
Post by Greg KH
Post by Theodore Ts'o
Post by g***@linuxfoundation.org
ext4: propagate errors up to ext4_find_entry()'s callers
I hate to do this again, but could you hold up this patch until the
next stable release?
want it to go in.
Hi Greg and other stable kernel maintainers,
Please cherry-pick the following commits from mainline into your
36de928 ext4: propagate errors up to ext4_find_entry()'s callers
a9cfcd6 ext4: avoid trying to kfree an ERR_PTR pointer
There was a bug accidentally introduced 36de928 which is fixed in
a9cfcd6, hence my request to Greg that he hold up the first commit.
These only applied to 3.16-stable, so I've applied them there.

thanks,

greg k-h
Ben Hutchings
2014-10-20 12:21:56 UTC
Permalink
Post by Theodore Ts'o
Post by Greg KH
Post by Theodore Ts'o
Post by g***@linuxfoundation.org
ext4: propagate errors up to ext4_find_entry()'s callers
I hate to do this again, but could you hold up this patch until the
next stable release?
want it to go in.
Hi Greg and other stable kernel maintainers,
Please cherry-pick the following commits from mainline into your
36de928 ext4: propagate errors up to ext4_find_entry()'s callers
This doesn't apply to 3.2. If it's needed, please provide a backport.

Ben.
Post by Theodore Ts'o
a9cfcd6 ext4: avoid trying to kfree an ERR_PTR pointer
There was a bug accidentally introduced 36de928 which is fixed in
a9cfcd6, hence my request to Greg that he hold up the first commit.
--
Ben Hutchings
[W]e found...that it wasn't as easy to get programs right as we had thought.
... I realized that a large part of my life from then on was going to be spent
in finding mistakes in my own programs. - Maurice Wilkes, 1949
Loading...