Re: isofs oops - d_splice_alias+0x1f (2.6.24-rc5-mm1)
On Sat, Jan 05, 2008 at 01:31:26AM -0800, Andrew Morton wrote: > On Thu, 3 Jan 2008 14:10:33 + Al Viro <[EMAIL PROTECTED]> wrote: > > > > > which would not manage to return ERR_PTR(-EIO), no matter what - it would > > die on access to inode->i_state. I don't have -mm tree at hand, check if > > there's anything affected in these areas. Perhaps somebody tried to pass > > error values from isofs_iget() and forgot to update callers? > > Yep. Unlike 2.6.24-rc5-mm1, 2.6.24-rc6-mm1 has > > iget-stop-isofs-from-using-read_inode-fix-2.patch > iget-stop-isofs-from-using-read_inode-fix-2-update.patch > > so hopefully this was all fixed. Nope; missed by both; fix (and fix to fix) had been posted, but IMO it's better to replace the entire sorry pile with new variant of patch dhowells had posted - it has all fixes folded in. -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: isofs oops - d_splice_alias+0x1f (2.6.24-rc5-mm1)
On Thu, 3 Jan 2008 14:10:33 + Al Viro <[EMAIL PROTECTED]> wrote: > > which would not manage to return ERR_PTR(-EIO), no matter what - it would > die on access to inode->i_state. I don't have -mm tree at hand, check if > there's anything affected in these areas. Perhaps somebody tried to pass > error values from isofs_iget() and forgot to update callers? Yep. Unlike 2.6.24-rc5-mm1, 2.6.24-rc6-mm1 has iget-stop-isofs-from-using-read_inode-fix-2.patch iget-stop-isofs-from-using-read_inode-fix-2-update.patch so hopefully this was all fixed. -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: isofs oops - d_splice_alias+0x1f (2.6.24-rc5-mm1)
On Thu, 3 Jan 2008 14:10:33 + Al Viro [EMAIL PROTECTED] wrote: which would not manage to return ERR_PTR(-EIO), no matter what - it would die on access to inode-i_state. I don't have -mm tree at hand, check if there's anything affected in these areas. Perhaps somebody tried to pass error values from isofs_iget() and forgot to update callers? Yep. Unlike 2.6.24-rc5-mm1, 2.6.24-rc6-mm1 has iget-stop-isofs-from-using-read_inode-fix-2.patch iget-stop-isofs-from-using-read_inode-fix-2-update.patch so hopefully this was all fixed. -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: isofs oops - d_splice_alias+0x1f (2.6.24-rc5-mm1)
On Sat, Jan 05, 2008 at 01:31:26AM -0800, Andrew Morton wrote: On Thu, 3 Jan 2008 14:10:33 + Al Viro [EMAIL PROTECTED] wrote: which would not manage to return ERR_PTR(-EIO), no matter what - it would die on access to inode-i_state. I don't have -mm tree at hand, check if there's anything affected in these areas. Perhaps somebody tried to pass error values from isofs_iget() and forgot to update callers? Yep. Unlike 2.6.24-rc5-mm1, 2.6.24-rc6-mm1 has iget-stop-isofs-from-using-read_inode-fix-2.patch iget-stop-isofs-from-using-read_inode-fix-2-update.patch so hopefully this was all fixed. Nope; missed by both; fix (and fix to fix) had been posted, but IMO it's better to replace the entire sorry pile with new variant of patch dhowells had posted - it has all fixes folded in. -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: isofs oops - d_splice_alias+0x1f (2.6.24-rc5-mm1)
* David Howells <[EMAIL PROTECTED]> wrote: > > FWIW, this patch pile is getting ridiculous - it's what, original + > > 2 fixes in -mm + mine + this one? Could you post the updated patch > > with all fixes and fixes to fixes folded into it? > > I can, though Andrew usually objects to that. folding often results in commit messages and hence credit lost - and thus discourages test/review feedback and bugfixing. It's also easier to see the track record/history of a patch if the fixes are in separate patches. It's also easier to undo a 'fix' (and track the state/version of changes) if it turns out to be a bad fix. If things get spaghetti then a new splitup indeed helps (for larger patchsets), and Andrew usually asks for a new splitup in that case. Andrew often folds patches together right before sending it off to Linus (while constructing combined credit - so credit is not lost). Ingo -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: isofs oops - d_splice_alias+0x1f (2.6.24-rc5-mm1)
Al Viro <[EMAIL PROTECTED]> wrote: > My apologies, should've had coffee before posting. Me too, probably. > FWIW, this patch pile is getting ridiculous - it's what, original + 2 fixes > in -mm + mine + this one? Could you post the updated patch with all fixes > and fixes to fixes folded into it? I can, though Andrew usually objects to that. David -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: isofs oops - d_splice_alias+0x1f (2.6.24-rc5-mm1)
On Fri, Jan 04, 2008 at 12:24:01PM +, David Howells wrote: > Add some more fixes to ISOFS error handling on top of Al Viro's patch: > > (1) Use IS_ERR() rather than ERR_PTR() to test for errors. *blush* My apologies, should've had coffee before posting. FWIW, this patch pile is getting ridiculous - it's what, original + 2 fixes in -mm + mine + this one? Could you post the updated patch with all fixes and fixes to fixes folded into it? -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: isofs oops - d_splice_alias+0x1f (2.6.24-rc5-mm1)
Al Viro <[EMAIL PROTECTED]> wrote:
> Not enough. isofs_iget() still can return NULL; that needs to be converted to
> ERR_PTR().
>
> BTW, ERR_CAST is there for purpose...
This patch should probably be added on top of that one.
David
---
IGET: Add another couple of fixes to ISOFS error handling
From: David Howells <[EMAIL PROTECTED]>
Add some more fixes to ISOFS error handling on top of Al Viro's patch:
(1) Use IS_ERR() rather than ERR_PTR() to test for errors.
(2) Return the error from isofs_iget() in parse_rock_ridge_inode_internal().
(3) In isofs_export_get_parent() return -ENOMEM if that was the error rather
than -EACCES. Should we return -EIO if that is the error rather than
-EACCES?
Signed-off-by: David Howells <[EMAIL PROTECTED]>
---
fs/isofs/export.c |8 +---
fs/isofs/namei.c |2 +-
fs/isofs/rock.c |4 +++-
3 files changed, 9 insertions(+), 5 deletions(-)
diff --git a/fs/isofs/export.c b/fs/isofs/export.c
index 63a2113..bb21913 100644
--- a/fs/isofs/export.c
+++ b/fs/isofs/export.c
@@ -26,7 +26,7 @@ isofs_export_iget(struct super_block *sb,
if (block == 0)
return ERR_PTR(-ESTALE);
inode = isofs_iget(sb, block, offset);
- if (ERR_PTR(inode))
+ if (IS_ERR(inode))
return ERR_CAST(inode);
if (generation && inode->i_generation != generation) {
iput(inode);
@@ -108,8 +108,10 @@ static struct dentry *isofs_export_get_parent(struct
dentry *child)
parent_inode = isofs_iget(child_inode->i_sb,
parent_block,
parent_offset);
- if (ERR_PTR(parent_inode)) {
- rv = ERR_PTR(-EACCES);
+ if (IS_ERR(parent_inode)) {
+ rv = ERR_CAST(parent_inode);
+ if (rv != ERR_PTR(-ENOMEM))
+ rv = ERR_PTR(-EACCES);
goto out;
}
diff --git a/fs/isofs/namei.c b/fs/isofs/namei.c
index beee021..344b247 100644
--- a/fs/isofs/namei.c
+++ b/fs/isofs/namei.c
@@ -179,7 +179,7 @@ struct dentry *isofs_lookup(struct inode *dir, struct
dentry *dentry, struct nam
inode = NULL;
if (found) {
inode = isofs_iget(dir->i_sb, block, offset);
- if (ERR_PTR(inode)) {
+ if (IS_ERR(inode)) {
unlock_kernel();
return ERR_CAST(inode);
}
diff --git a/fs/isofs/rock.c b/fs/isofs/rock.c
index c62b650..6bd48f0 100644
--- a/fs/isofs/rock.c
+++ b/fs/isofs/rock.c
@@ -474,8 +474,10 @@ repeat:
isofs_iget(inode->i_sb,
ISOFS_I(inode)->i_first_extent,
0);
- if (ERR_PTR(reloc))
+ if (IS_ERR(reloc)) {
+ ret = PTR_ERR(reloc);
goto out;
+ }
inode->i_mode = reloc->i_mode;
inode->i_nlink = reloc->i_nlink;
inode->i_uid = reloc->i_uid;
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: isofs oops - d_splice_alias+0x1f (2.6.24-rc5-mm1)
On Fri, Jan 04, 2008 at 07:13:57PM +0800, Dave Young wrote:
> > iget-stop-isofs-from-using-read_inode-fix*
> >
> > Callers in fs/isofs/namei.c are missed.
>
> Yes, some isofs_iget return value should be handled, it is missed.
>
> Maybe this:
Not enough. isofs_iget() still can return NULL; that needs to be converted to
ERR_PTR().
BTW, ERR_CAST is there for purpose...
Signed-off-by: Al Viro <[EMAIL PROTECTED]>
---
diff -urN foo1/fs/isofs/export.c foo2/fs/isofs/export.c
--- foo1/fs/isofs/export.c 2008-01-04 06:23:10.0 -0500
+++ foo2/fs/isofs/export.c 2008-01-04 06:18:52.0 -0500
@@ -26,11 +26,9 @@
if (block == 0)
return ERR_PTR(-ESTALE);
inode = isofs_iget(sb, block, offset);
- if (inode == NULL)
- return ERR_PTR(-ENOMEM);
- if (is_bad_inode(inode)
- || (generation && inode->i_generation != generation))
- {
+ if (ERR_PTR(inode))
+ return ERR_CAST(inode);
+ if (generation && inode->i_generation != generation) {
iput(inode);
return ERR_PTR(-ESTALE);
}
@@ -110,7 +108,7 @@
parent_inode = isofs_iget(child_inode->i_sb,
parent_block,
parent_offset);
- if (parent_inode == NULL) {
+ if (ERR_PTR(parent_inode)) {
rv = ERR_PTR(-EACCES);
goto out;
}
diff -urN foo1/fs/isofs/inode.c foo2/fs/isofs/inode.c
--- foo1/fs/isofs/inode.c 2008-01-04 06:23:33.0 -0500
+++ foo2/fs/isofs/inode.c 2008-01-04 06:15:43.0 -0500
@@ -1408,7 +1408,7 @@
long ret;
if (offset >= 1ul << sb->s_blocksize_bits)
- return NULL;
+ return ERR_PTR(-EINVAL);
data.block = block;
data.offset = offset;
@@ -1418,7 +1418,10 @@
inode = iget5_locked(sb, hashval, _iget5_test,
_iget5_set, );
- if (inode && (inode->i_state & I_NEW)) {
+ if (!inode)
+ return ERR_PTR(-ENOMEM);
+
+ if (inode->i_state & I_NEW) {
ret = isofs_read_inode(inode);
if (ret < 0) {
iget_failed(inode);
diff -urN foo1/fs/isofs/namei.c foo2/fs/isofs/namei.c
--- foo1/fs/isofs/namei.c 2008-01-04 06:23:10.0 -0500
+++ foo2/fs/isofs/namei.c 2008-01-04 06:19:20.0 -0500
@@ -179,9 +179,9 @@
inode = NULL;
if (found) {
inode = isofs_iget(dir->i_sb, block, offset);
- if (!inode) {
+ if (ERR_PTR(inode)) {
unlock_kernel();
- return ERR_PTR(-EACCES);
+ return ERR_CAST(inode);
}
}
unlock_kernel();
diff -urN foo1/fs/isofs/rock.c foo2/fs/isofs/rock.c
--- foo1/fs/isofs/rock.c2008-01-04 06:23:10.0 -0500
+++ foo2/fs/isofs/rock.c2008-01-04 06:20:41.0 -0500
@@ -474,7 +474,7 @@
isofs_iget(inode->i_sb,
ISOFS_I(inode)->i_first_extent,
0);
- if (!reloc)
+ if (ERR_PTR(reloc))
goto out;
inode->i_mode = reloc->i_mode;
inode->i_nlink = reloc->i_nlink;
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: isofs oops - d_splice_alias+0x1f (2.6.24-rc5-mm1)
On Fri, Jan 04, 2008 at 07:13:57PM +0800, Dave Young wrote:
> On Fri, Jan 04, 2008 at 10:47:55AM +, Al Viro wrote:
> > On Thu, Jan 03, 2008 at 03:15:56PM +0100, Jiri Slaby wrote:
> >
> > > Can't say, the DVD seems to be OK, I don't know what was wrong (as I can
> > > say,
> > > this happened several times in the past yet and after reboot everything
> > > OK; I
> > > suspect gnome auto mounter -- multiple machines, several DVD ROMs, same
> > > disk
> > > with OS, similar errors, but that's too few to report).
> >
> > It is -mm-specific, all right - fallout from
> > iget-stop-isofs-from-using-read_inode.patch
> > not covered in
> > iget-stop-isofs-from-using-read_inode-fix*
> >
> > Callers in fs/isofs/namei.c are missed.
>
> Yes, some isofs_iget return value should be handled, it is missed.
>
> Maybe this:
>
> Signed-off-by: Dave Young <[EMAIL PROTECTED]>
>
> ---
> fs/isofs/export.c |8
> fs/isofs/namei.c |4 ++--
> fs/isofs/rock.c |4 +++-
> 3 files changed, 9 insertions(+), 7 deletions(-)
>
[snip]
I'm sorry for patch of building error, following one instead:
Signed-off-by: Dave Young <[EMAIL PROTECTED]>
---
fs/isofs/export.c |8
fs/isofs/namei.c |4 ++--
fs/isofs/rock.c |4 +++-
3 files changed, 9 insertions(+), 7 deletions(-)
diff -upr linux/fs/isofs/export.c linux.new/fs/isofs/export.c
--- linux/fs/isofs/export.c 2008-01-04 18:54:40.0 +0800
+++ linux.new/fs/isofs/export.c 2008-01-04 19:18:45.0 +0800
@@ -26,8 +26,8 @@ isofs_export_iget(struct super_block *sb
if (block == 0)
return ERR_PTR(-ESTALE);
inode = isofs_iget(sb, block, offset);
- if (inode == NULL)
- return ERR_PTR(-ENOMEM);
+ if (IS_ERR(inode))
+ return (struct dentry *)inode;
if (is_bad_inode(inode)
|| (generation && inode->i_generation != generation))
{
@@ -110,8 +110,8 @@ static struct dentry *isofs_export_get_p
parent_inode = isofs_iget(child_inode->i_sb,
parent_block,
parent_offset);
- if (parent_inode == NULL) {
- rv = ERR_PTR(-EACCES);
+ if (IS_ERR(parent_inode)) {
+ rv = (struct dentry *)parent_inode;
goto out;
}
diff -upr linux/fs/isofs/namei.c linux.new/fs/isofs/namei.c
--- linux/fs/isofs/namei.c 2008-01-04 18:54:40.0 +0800
+++ linux.new/fs/isofs/namei.c 2008-01-04 19:18:45.0 +0800
@@ -179,9 +179,9 @@ struct dentry *isofs_lookup(struct inode
inode = NULL;
if (found) {
inode = isofs_iget(dir->i_sb, block, offset);
- if (!inode) {
+ if (IS_ERR(inode)) {
unlock_kernel();
- return ERR_PTR(-EACCES);
+ return inode;
}
}
unlock_kernel();
diff -upr linux/fs/isofs/rock.c linux.new/fs/isofs/rock.c
--- linux/fs/isofs/rock.c 2008-01-04 18:54:40.0 +0800
+++ linux.new/fs/isofs/rock.c 2008-01-04 19:18:45.0 +0800
@@ -474,8 +474,10 @@ repeat:
isofs_iget(inode->i_sb,
ISOFS_I(inode)->i_first_extent,
0);
- if (!reloc)
+ if (IS_ERR(reloc)) {
+ ret = PTR_ERR(reloc);
goto out;
+ }
inode->i_mode = reloc->i_mode;
inode->i_nlink = reloc->i_nlink;
inode->i_uid = reloc->i_uid;
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: isofs oops - d_splice_alias+0x1f (2.6.24-rc5-mm1)
On Fri, Jan 04, 2008 at 10:47:55AM +, Al Viro wrote:
> On Thu, Jan 03, 2008 at 03:15:56PM +0100, Jiri Slaby wrote:
>
> > Can't say, the DVD seems to be OK, I don't know what was wrong (as I can
> > say,
> > this happened several times in the past yet and after reboot everything OK;
> > I
> > suspect gnome auto mounter -- multiple machines, several DVD ROMs, same disk
> > with OS, similar errors, but that's too few to report).
>
> It is -mm-specific, all right - fallout from
> iget-stop-isofs-from-using-read_inode.patch
> not covered in
> iget-stop-isofs-from-using-read_inode-fix*
>
> Callers in fs/isofs/namei.c are missed.
Yes, some isofs_iget return value should be handled, it is missed.
Maybe this:
Signed-off-by: Dave Young <[EMAIL PROTECTED]>
---
fs/isofs/export.c |8
fs/isofs/namei.c |4 ++--
fs/isofs/rock.c |4 +++-
3 files changed, 9 insertions(+), 7 deletions(-)
diff -upr linux/fs/isofs/export.c linux.new/fs/isofs/export.c
--- linux/fs/isofs/export.c 2008-01-04 18:54:40.0 +0800
+++ linux.new/fs/isofs/export.c 2008-01-04 19:04:31.0 +0800
@@ -26,8 +26,8 @@ isofs_export_iget(struct super_block *sb
if (block == 0)
return ERR_PTR(-ESTALE);
inode = isofs_iget(sb, block, offset);
- if (inode == NULL)
- return ERR_PTR(-ENOMEM);
+ if (IS_ERR(inode))
+ return inode;
if (is_bad_inode(inode)
|| (generation && inode->i_generation != generation))
{
@@ -110,8 +110,8 @@ static struct dentry *isofs_export_get_p
parent_inode = isofs_iget(child_inode->i_sb,
parent_block,
parent_offset);
- if (parent_inode == NULL) {
- rv = ERR_PTR(-EACCES);
+ if (IS_ERR(parent_inode)) {
+ rv = parent_inode;
goto out;
}
diff -upr linux/fs/isofs/namei.c linux.new/fs/isofs/namei.c
--- linux/fs/isofs/namei.c 2008-01-04 18:54:40.0 +0800
+++ linux.new/fs/isofs/namei.c 2008-01-04 18:57:27.0 +0800
@@ -179,9 +179,9 @@ struct dentry *isofs_lookup(struct inode
inode = NULL;
if (found) {
inode = isofs_iget(dir->i_sb, block, offset);
- if (!inode) {
+ if (IS_ERR(inode)) {
unlock_kernel();
- return ERR_PTR(-EACCES);
+ return inode;
}
}
unlock_kernel();
diff -upr linux/fs/isofs/rock.c linux.new/fs/isofs/rock.c
--- linux/fs/isofs/rock.c 2008-01-04 18:54:40.0 +0800
+++ linux.new/fs/isofs/rock.c 2008-01-04 19:01:10.0 +0800
@@ -474,8 +474,10 @@ repeat:
isofs_iget(inode->i_sb,
ISOFS_I(inode)->i_first_extent,
0);
- if (!reloc)
+ if (IS_ERR(reloc)) {
+ err = PTR_ERR(reloc);
goto out;
+ }
inode->i_mode = reloc->i_mode;
inode->i_nlink = reloc->i_nlink;
inode->i_uid = reloc->i_uid;
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: isofs oops - d_splice_alias+0x1f (2.6.24-rc5-mm1)
On Thu, Jan 03, 2008 at 03:15:56PM +0100, Jiri Slaby wrote: > Can't say, the DVD seems to be OK, I don't know what was wrong (as I can say, > this happened several times in the past yet and after reboot everything OK; I > suspect gnome auto mounter -- multiple machines, several DVD ROMs, same disk > with OS, similar errors, but that's too few to report). It is -mm-specific, all right - fallout from iget-stop-isofs-from-using-read_inode.patch not covered in iget-stop-isofs-from-using-read_inode-fix* Callers in fs/isofs/namei.c are missed. -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: isofs oops - d_splice_alias+0x1f (2.6.24-rc5-mm1)
On Fri, Jan 04, 2008 at 07:13:57PM +0800, Dave Young wrote:
iget-stop-isofs-from-using-read_inode-fix*
Callers in fs/isofs/namei.c are missed.
Yes, some isofs_iget return value should be handled, it is missed.
Maybe this:
Not enough. isofs_iget() still can return NULL; that needs to be converted to
ERR_PTR().
BTW, ERR_CAST is there for purpose...
Signed-off-by: Al Viro [EMAIL PROTECTED]
---
diff -urN foo1/fs/isofs/export.c foo2/fs/isofs/export.c
--- foo1/fs/isofs/export.c 2008-01-04 06:23:10.0 -0500
+++ foo2/fs/isofs/export.c 2008-01-04 06:18:52.0 -0500
@@ -26,11 +26,9 @@
if (block == 0)
return ERR_PTR(-ESTALE);
inode = isofs_iget(sb, block, offset);
- if (inode == NULL)
- return ERR_PTR(-ENOMEM);
- if (is_bad_inode(inode)
- || (generation inode-i_generation != generation))
- {
+ if (ERR_PTR(inode))
+ return ERR_CAST(inode);
+ if (generation inode-i_generation != generation) {
iput(inode);
return ERR_PTR(-ESTALE);
}
@@ -110,7 +108,7 @@
parent_inode = isofs_iget(child_inode-i_sb,
parent_block,
parent_offset);
- if (parent_inode == NULL) {
+ if (ERR_PTR(parent_inode)) {
rv = ERR_PTR(-EACCES);
goto out;
}
diff -urN foo1/fs/isofs/inode.c foo2/fs/isofs/inode.c
--- foo1/fs/isofs/inode.c 2008-01-04 06:23:33.0 -0500
+++ foo2/fs/isofs/inode.c 2008-01-04 06:15:43.0 -0500
@@ -1408,7 +1408,7 @@
long ret;
if (offset = 1ul sb-s_blocksize_bits)
- return NULL;
+ return ERR_PTR(-EINVAL);
data.block = block;
data.offset = offset;
@@ -1418,7 +1418,10 @@
inode = iget5_locked(sb, hashval, isofs_iget5_test,
isofs_iget5_set, data);
- if (inode (inode-i_state I_NEW)) {
+ if (!inode)
+ return ERR_PTR(-ENOMEM);
+
+ if (inode-i_state I_NEW) {
ret = isofs_read_inode(inode);
if (ret 0) {
iget_failed(inode);
diff -urN foo1/fs/isofs/namei.c foo2/fs/isofs/namei.c
--- foo1/fs/isofs/namei.c 2008-01-04 06:23:10.0 -0500
+++ foo2/fs/isofs/namei.c 2008-01-04 06:19:20.0 -0500
@@ -179,9 +179,9 @@
inode = NULL;
if (found) {
inode = isofs_iget(dir-i_sb, block, offset);
- if (!inode) {
+ if (ERR_PTR(inode)) {
unlock_kernel();
- return ERR_PTR(-EACCES);
+ return ERR_CAST(inode);
}
}
unlock_kernel();
diff -urN foo1/fs/isofs/rock.c foo2/fs/isofs/rock.c
--- foo1/fs/isofs/rock.c2008-01-04 06:23:10.0 -0500
+++ foo2/fs/isofs/rock.c2008-01-04 06:20:41.0 -0500
@@ -474,7 +474,7 @@
isofs_iget(inode-i_sb,
ISOFS_I(inode)-i_first_extent,
0);
- if (!reloc)
+ if (ERR_PTR(reloc))
goto out;
inode-i_mode = reloc-i_mode;
inode-i_nlink = reloc-i_nlink;
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: isofs oops - d_splice_alias+0x1f (2.6.24-rc5-mm1)
On Fri, Jan 04, 2008 at 07:13:57PM +0800, Dave Young wrote:
On Fri, Jan 04, 2008 at 10:47:55AM +, Al Viro wrote:
On Thu, Jan 03, 2008 at 03:15:56PM +0100, Jiri Slaby wrote:
Can't say, the DVD seems to be OK, I don't know what was wrong (as I can
say,
this happened several times in the past yet and after reboot everything
OK; I
suspect gnome auto mounter -- multiple machines, several DVD ROMs, same
disk
with OS, similar errors, but that's too few to report).
It is -mm-specific, all right - fallout from
iget-stop-isofs-from-using-read_inode.patch
not covered in
iget-stop-isofs-from-using-read_inode-fix*
Callers in fs/isofs/namei.c are missed.
Yes, some isofs_iget return value should be handled, it is missed.
Maybe this:
Signed-off-by: Dave Young [EMAIL PROTECTED]
---
fs/isofs/export.c |8
fs/isofs/namei.c |4 ++--
fs/isofs/rock.c |4 +++-
3 files changed, 9 insertions(+), 7 deletions(-)
[snip]
I'm sorry for patch of building error, following one instead:
Signed-off-by: Dave Young [EMAIL PROTECTED]
---
fs/isofs/export.c |8
fs/isofs/namei.c |4 ++--
fs/isofs/rock.c |4 +++-
3 files changed, 9 insertions(+), 7 deletions(-)
diff -upr linux/fs/isofs/export.c linux.new/fs/isofs/export.c
--- linux/fs/isofs/export.c 2008-01-04 18:54:40.0 +0800
+++ linux.new/fs/isofs/export.c 2008-01-04 19:18:45.0 +0800
@@ -26,8 +26,8 @@ isofs_export_iget(struct super_block *sb
if (block == 0)
return ERR_PTR(-ESTALE);
inode = isofs_iget(sb, block, offset);
- if (inode == NULL)
- return ERR_PTR(-ENOMEM);
+ if (IS_ERR(inode))
+ return (struct dentry *)inode;
if (is_bad_inode(inode)
|| (generation inode-i_generation != generation))
{
@@ -110,8 +110,8 @@ static struct dentry *isofs_export_get_p
parent_inode = isofs_iget(child_inode-i_sb,
parent_block,
parent_offset);
- if (parent_inode == NULL) {
- rv = ERR_PTR(-EACCES);
+ if (IS_ERR(parent_inode)) {
+ rv = (struct dentry *)parent_inode;
goto out;
}
diff -upr linux/fs/isofs/namei.c linux.new/fs/isofs/namei.c
--- linux/fs/isofs/namei.c 2008-01-04 18:54:40.0 +0800
+++ linux.new/fs/isofs/namei.c 2008-01-04 19:18:45.0 +0800
@@ -179,9 +179,9 @@ struct dentry *isofs_lookup(struct inode
inode = NULL;
if (found) {
inode = isofs_iget(dir-i_sb, block, offset);
- if (!inode) {
+ if (IS_ERR(inode)) {
unlock_kernel();
- return ERR_PTR(-EACCES);
+ return inode;
}
}
unlock_kernel();
diff -upr linux/fs/isofs/rock.c linux.new/fs/isofs/rock.c
--- linux/fs/isofs/rock.c 2008-01-04 18:54:40.0 +0800
+++ linux.new/fs/isofs/rock.c 2008-01-04 19:18:45.0 +0800
@@ -474,8 +474,10 @@ repeat:
isofs_iget(inode-i_sb,
ISOFS_I(inode)-i_first_extent,
0);
- if (!reloc)
+ if (IS_ERR(reloc)) {
+ ret = PTR_ERR(reloc);
goto out;
+ }
inode-i_mode = reloc-i_mode;
inode-i_nlink = reloc-i_nlink;
inode-i_uid = reloc-i_uid;
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: isofs oops - d_splice_alias+0x1f (2.6.24-rc5-mm1)
On Fri, Jan 04, 2008 at 10:47:55AM +, Al Viro wrote:
On Thu, Jan 03, 2008 at 03:15:56PM +0100, Jiri Slaby wrote:
Can't say, the DVD seems to be OK, I don't know what was wrong (as I can
say,
this happened several times in the past yet and after reboot everything OK;
I
suspect gnome auto mounter -- multiple machines, several DVD ROMs, same disk
with OS, similar errors, but that's too few to report).
It is -mm-specific, all right - fallout from
iget-stop-isofs-from-using-read_inode.patch
not covered in
iget-stop-isofs-from-using-read_inode-fix*
Callers in fs/isofs/namei.c are missed.
Yes, some isofs_iget return value should be handled, it is missed.
Maybe this:
Signed-off-by: Dave Young [EMAIL PROTECTED]
---
fs/isofs/export.c |8
fs/isofs/namei.c |4 ++--
fs/isofs/rock.c |4 +++-
3 files changed, 9 insertions(+), 7 deletions(-)
diff -upr linux/fs/isofs/export.c linux.new/fs/isofs/export.c
--- linux/fs/isofs/export.c 2008-01-04 18:54:40.0 +0800
+++ linux.new/fs/isofs/export.c 2008-01-04 19:04:31.0 +0800
@@ -26,8 +26,8 @@ isofs_export_iget(struct super_block *sb
if (block == 0)
return ERR_PTR(-ESTALE);
inode = isofs_iget(sb, block, offset);
- if (inode == NULL)
- return ERR_PTR(-ENOMEM);
+ if (IS_ERR(inode))
+ return inode;
if (is_bad_inode(inode)
|| (generation inode-i_generation != generation))
{
@@ -110,8 +110,8 @@ static struct dentry *isofs_export_get_p
parent_inode = isofs_iget(child_inode-i_sb,
parent_block,
parent_offset);
- if (parent_inode == NULL) {
- rv = ERR_PTR(-EACCES);
+ if (IS_ERR(parent_inode)) {
+ rv = parent_inode;
goto out;
}
diff -upr linux/fs/isofs/namei.c linux.new/fs/isofs/namei.c
--- linux/fs/isofs/namei.c 2008-01-04 18:54:40.0 +0800
+++ linux.new/fs/isofs/namei.c 2008-01-04 18:57:27.0 +0800
@@ -179,9 +179,9 @@ struct dentry *isofs_lookup(struct inode
inode = NULL;
if (found) {
inode = isofs_iget(dir-i_sb, block, offset);
- if (!inode) {
+ if (IS_ERR(inode)) {
unlock_kernel();
- return ERR_PTR(-EACCES);
+ return inode;
}
}
unlock_kernel();
diff -upr linux/fs/isofs/rock.c linux.new/fs/isofs/rock.c
--- linux/fs/isofs/rock.c 2008-01-04 18:54:40.0 +0800
+++ linux.new/fs/isofs/rock.c 2008-01-04 19:01:10.0 +0800
@@ -474,8 +474,10 @@ repeat:
isofs_iget(inode-i_sb,
ISOFS_I(inode)-i_first_extent,
0);
- if (!reloc)
+ if (IS_ERR(reloc)) {
+ err = PTR_ERR(reloc);
goto out;
+ }
inode-i_mode = reloc-i_mode;
inode-i_nlink = reloc-i_nlink;
inode-i_uid = reloc-i_uid;
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: isofs oops - d_splice_alias+0x1f (2.6.24-rc5-mm1)
On Thu, Jan 03, 2008 at 03:15:56PM +0100, Jiri Slaby wrote: Can't say, the DVD seems to be OK, I don't know what was wrong (as I can say, this happened several times in the past yet and after reboot everything OK; I suspect gnome auto mounter -- multiple machines, several DVD ROMs, same disk with OS, similar errors, but that's too few to report). It is -mm-specific, all right - fallout from iget-stop-isofs-from-using-read_inode.patch not covered in iget-stop-isofs-from-using-read_inode-fix* Callers in fs/isofs/namei.c are missed. -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: isofs oops - d_splice_alias+0x1f (2.6.24-rc5-mm1)
Al Viro [EMAIL PROTECTED] wrote:
Not enough. isofs_iget() still can return NULL; that needs to be converted to
ERR_PTR().
BTW, ERR_CAST is there for purpose...
This patch should probably be added on top of that one.
David
---
IGET: Add another couple of fixes to ISOFS error handling
From: David Howells [EMAIL PROTECTED]
Add some more fixes to ISOFS error handling on top of Al Viro's patch:
(1) Use IS_ERR() rather than ERR_PTR() to test for errors.
(2) Return the error from isofs_iget() in parse_rock_ridge_inode_internal().
(3) In isofs_export_get_parent() return -ENOMEM if that was the error rather
than -EACCES. Should we return -EIO if that is the error rather than
-EACCES?
Signed-off-by: David Howells [EMAIL PROTECTED]
---
fs/isofs/export.c |8 +---
fs/isofs/namei.c |2 +-
fs/isofs/rock.c |4 +++-
3 files changed, 9 insertions(+), 5 deletions(-)
diff --git a/fs/isofs/export.c b/fs/isofs/export.c
index 63a2113..bb21913 100644
--- a/fs/isofs/export.c
+++ b/fs/isofs/export.c
@@ -26,7 +26,7 @@ isofs_export_iget(struct super_block *sb,
if (block == 0)
return ERR_PTR(-ESTALE);
inode = isofs_iget(sb, block, offset);
- if (ERR_PTR(inode))
+ if (IS_ERR(inode))
return ERR_CAST(inode);
if (generation inode-i_generation != generation) {
iput(inode);
@@ -108,8 +108,10 @@ static struct dentry *isofs_export_get_parent(struct
dentry *child)
parent_inode = isofs_iget(child_inode-i_sb,
parent_block,
parent_offset);
- if (ERR_PTR(parent_inode)) {
- rv = ERR_PTR(-EACCES);
+ if (IS_ERR(parent_inode)) {
+ rv = ERR_CAST(parent_inode);
+ if (rv != ERR_PTR(-ENOMEM))
+ rv = ERR_PTR(-EACCES);
goto out;
}
diff --git a/fs/isofs/namei.c b/fs/isofs/namei.c
index beee021..344b247 100644
--- a/fs/isofs/namei.c
+++ b/fs/isofs/namei.c
@@ -179,7 +179,7 @@ struct dentry *isofs_lookup(struct inode *dir, struct
dentry *dentry, struct nam
inode = NULL;
if (found) {
inode = isofs_iget(dir-i_sb, block, offset);
- if (ERR_PTR(inode)) {
+ if (IS_ERR(inode)) {
unlock_kernel();
return ERR_CAST(inode);
}
diff --git a/fs/isofs/rock.c b/fs/isofs/rock.c
index c62b650..6bd48f0 100644
--- a/fs/isofs/rock.c
+++ b/fs/isofs/rock.c
@@ -474,8 +474,10 @@ repeat:
isofs_iget(inode-i_sb,
ISOFS_I(inode)-i_first_extent,
0);
- if (ERR_PTR(reloc))
+ if (IS_ERR(reloc)) {
+ ret = PTR_ERR(reloc);
goto out;
+ }
inode-i_mode = reloc-i_mode;
inode-i_nlink = reloc-i_nlink;
inode-i_uid = reloc-i_uid;
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: isofs oops - d_splice_alias+0x1f (2.6.24-rc5-mm1)
On Fri, Jan 04, 2008 at 12:24:01PM +, David Howells wrote: Add some more fixes to ISOFS error handling on top of Al Viro's patch: (1) Use IS_ERR() rather than ERR_PTR() to test for errors. *blush* My apologies, should've had coffee before posting. FWIW, this patch pile is getting ridiculous - it's what, original + 2 fixes in -mm + mine + this one? Could you post the updated patch with all fixes and fixes to fixes folded into it? -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: isofs oops - d_splice_alias+0x1f (2.6.24-rc5-mm1)
Al Viro [EMAIL PROTECTED] wrote: My apologies, should've had coffee before posting. Me too, probably. FWIW, this patch pile is getting ridiculous - it's what, original + 2 fixes in -mm + mine + this one? Could you post the updated patch with all fixes and fixes to fixes folded into it? I can, though Andrew usually objects to that. David -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: isofs oops - d_splice_alias+0x1f (2.6.24-rc5-mm1)
On 01/03/2008 02:51 PM, Pekka J Enberg wrote:
> Hi Jiri,
>
> On Thu, 3 Jan 2008, Jiri Slaby wrote:
>> this happened, while playing with broken dvd.
>
> [snip]
>
>> Buffer I/O error on device sr0, logical block 5441
>> end_request: I/O error, dev sr0, sector 136
>> ISOFS: unable to read i-node block
>> Unable to handle kernel NULL pointer dereference at 00ad RIP:
>> [] d_splice_alias+0x1f/0x100
>
> [snip]
>
>> Call Trace:
>> [] :isofs:isofs_lookup+0x395/0x4a0
>> [] d_alloc+0x2b/0x1d0
>> [] do_lookup+0x1ac/0x200
>
> Does the following patch fix it?
>
> Pekka
>
> [PATCH] isofs: check for bad inode in isofs_lookup
> From: Pekka Enberg <[EMAIL PROTECTED]>
>
> If isofs_read_inode() fails to read one of the inode blocks from disk, it
> returns a bad inode.
>
> Signed-off-by: Pekka Enberg <[EMAIL PROTECTED]>
> ---
> fs/isofs/namei.c |5 +
> 1 file changed, 5 insertions(+)
>
> Index: linux-2.6/fs/isofs/namei.c
> ===
> --- linux-2.6.orig/fs/isofs/namei.c
> +++ linux-2.6/fs/isofs/namei.c
> @@ -183,6 +183,11 @@ struct dentry *isofs_lookup(struct inode
> unlock_kernel();
> return ERR_PTR(-EACCES);
> }
> + if (is_bad_inode(inode)) {
> + unlock_kernel();
> + iput(inode);
> + return ERR_PTR(-ENOENT);
> + }
> }
> unlock_kernel();
> return d_splice_alias(inode, dentry);
Can't say, the DVD seems to be OK, I don't know what was wrong (as I can say,
this happened several times in the past yet and after reboot everything OK; I
suspect gnome auto mounter -- multiple machines, several DVD ROMs, same disk
with OS, similar errors, but that's too few to report).
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: isofs oops - d_splice_alias+0x1f (2.6.24-rc5-mm1)
On Thu, Jan 03, 2008 at 03:11:20PM +0100, Ingo Molnar wrote: > and there are about 5 other callsites as well that only check for a NULL > return. ... except that this is *not* getting NULL or make_bad_inode(); that's getting ERR_PTR(), which can't happen in mainline. So that looks like an -mm patch changing calling conventions and not updating callers... -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: isofs oops - d_splice_alias+0x1f (2.6.24-rc5-mm1)
* Pekka J Enberg <[EMAIL PROTECTED]> wrote:
> return ERR_PTR(-EACCES);
> }
> + if (is_bad_inode(inode)) {
> + unlock_kernel();
> + iput(inode);
> + return ERR_PTR(-ENOENT);
> + }
fs/isofs/rock.c:474 parse_rock_ridge_inode_internal() seems buggy too:
reloc =
isofs_iget(inode->i_sb,
ISOFS_I(inode)->i_first_extent,
0);
if (!reloc)
goto out;
it should probably do "!reloc || is_bad_inode(inode)" as well.
and there are about 5 other callsites as well that only check for a NULL
return.
perhaps the better fix would be to add this to inode.c:isofs_iget():
if (inode && (inode->i_state & I_NEW)) {
sb->s_op->read_inode(inode);
unlock_new_inode(inode);
if (s_bad_inode(inode))
inode = NULL;
}
?
Ingo
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: isofs oops - d_splice_alias+0x1f (2.6.24-rc5-mm1)
On Thu, Jan 03, 2008 at 03:51:28PM +0200, Pekka J Enberg wrote: > Does the following patch fix it? Highly doubtful, and doesn't deal with the real problem, AFAICS... -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: isofs oops - d_splice_alias+0x1f (2.6.24-rc5-mm1)
On Thu, Jan 03, 2008 at 02:23:44PM +0100, Jiri Slaby wrote:
> ISOFS: unable to read i-node block
isofs_read_inode() failing, about to do make_bad_inode()
> Unable to handle kernel NULL pointer dereference at 00ad RIP:
> [] d_splice_alias+0x1f/0x100
struct dentry *d_splice_alias(struct inode *inode, struct dentry *dentry)
{
struct dentry *new = NULL;
if (inode && S_ISDIR(inode->i_mode)) {
^
> RIP: 0010:[] [] d_splice_alias+0x1f/0x100
> RSP: :810061543b08 EFLAGS: 00010282
> RAX: RBX: fffb RCX: 880d3454
> RDX: 810048542750 RSI: 81005e114e10 RDI: fffb
with inode equal to (struct inode *)-5. Which is ERR_PTR(-EIO)...
> [] :isofs:isofs_lookup+0x395/0x4a0
inode = NULL;
if (found) {
inode = isofs_iget(dir->i_sb, block, offset);
if (!inode) {
unlock_kernel();
return ERR_PTR(-EACCES);
}
}
unlock_kernel();
return d_splice_alias(inode, dentry);
So we've got ERR_PTR(-EIO) from isofs_iget(). Bloody odd, seeing that
isofs_iget() either explicitly returns NULL or does
if (inode && (inode->i_state & I_NEW)) {
sb->s_op->read_inode(inode);
unlock_new_inode(inode);
}
return inode;
which would not manage to return ERR_PTR(-EIO), no matter what - it would
die on access to inode->i_state. I don't have -mm tree at hand, check if
there's anything affected in these areas. Perhaps somebody tried to pass
error values from isofs_iget() and forgot to update callers?
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: isofs oops - d_splice_alias+0x1f (2.6.24-rc5-mm1)
Hi Jiri,
On Thu, 3 Jan 2008, Jiri Slaby wrote:
> this happened, while playing with broken dvd.
[snip]
> Buffer I/O error on device sr0, logical block 5441
> end_request: I/O error, dev sr0, sector 136
> ISOFS: unable to read i-node block
> Unable to handle kernel NULL pointer dereference at 00ad RIP:
> [] d_splice_alias+0x1f/0x100
[snip]
> Call Trace:
> [] :isofs:isofs_lookup+0x395/0x4a0
> [] d_alloc+0x2b/0x1d0
> [] do_lookup+0x1ac/0x200
Does the following patch fix it?
Pekka
[PATCH] isofs: check for bad inode in isofs_lookup
From: Pekka Enberg <[EMAIL PROTECTED]>
If isofs_read_inode() fails to read one of the inode blocks from disk, it
returns a bad inode.
Signed-off-by: Pekka Enberg <[EMAIL PROTECTED]>
---
fs/isofs/namei.c |5 +
1 file changed, 5 insertions(+)
Index: linux-2.6/fs/isofs/namei.c
===
--- linux-2.6.orig/fs/isofs/namei.c
+++ linux-2.6/fs/isofs/namei.c
@@ -183,6 +183,11 @@ struct dentry *isofs_lookup(struct inode
unlock_kernel();
return ERR_PTR(-EACCES);
}
+ if (is_bad_inode(inode)) {
+ unlock_kernel();
+ iput(inode);
+ return ERR_PTR(-ENOENT);
+ }
}
unlock_kernel();
return d_splice_alias(inode, dentry);
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: isofs oops - d_splice_alias+0x1f (2.6.24-rc5-mm1)
Hi Jiri,
On Thu, 3 Jan 2008, Jiri Slaby wrote:
this happened, while playing with broken dvd.
[snip]
Buffer I/O error on device sr0, logical block 5441
end_request: I/O error, dev sr0, sector 136
ISOFS: unable to read i-node block
Unable to handle kernel NULL pointer dereference at 00ad RIP:
[802a679f] d_splice_alias+0x1f/0x100
[snip]
Call Trace:
[880d2395] :isofs:isofs_lookup+0x395/0x4a0
[802a565b] d_alloc+0x2b/0x1d0
[8029a97c] do_lookup+0x1ac/0x200
Does the following patch fix it?
Pekka
[PATCH] isofs: check for bad inode in isofs_lookup
From: Pekka Enberg [EMAIL PROTECTED]
If isofs_read_inode() fails to read one of the inode blocks from disk, it
returns a bad inode.
Signed-off-by: Pekka Enberg [EMAIL PROTECTED]
---
fs/isofs/namei.c |5 +
1 file changed, 5 insertions(+)
Index: linux-2.6/fs/isofs/namei.c
===
--- linux-2.6.orig/fs/isofs/namei.c
+++ linux-2.6/fs/isofs/namei.c
@@ -183,6 +183,11 @@ struct dentry *isofs_lookup(struct inode
unlock_kernel();
return ERR_PTR(-EACCES);
}
+ if (is_bad_inode(inode)) {
+ unlock_kernel();
+ iput(inode);
+ return ERR_PTR(-ENOENT);
+ }
}
unlock_kernel();
return d_splice_alias(inode, dentry);
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: isofs oops - d_splice_alias+0x1f (2.6.24-rc5-mm1)
On Thu, Jan 03, 2008 at 02:23:44PM +0100, Jiri Slaby wrote:
ISOFS: unable to read i-node block
isofs_read_inode() failing, about to do make_bad_inode()
Unable to handle kernel NULL pointer dereference at 00ad RIP:
[802a679f] d_splice_alias+0x1f/0x100
struct dentry *d_splice_alias(struct inode *inode, struct dentry *dentry)
{
struct dentry *new = NULL;
if (inode S_ISDIR(inode-i_mode)) {
^
RIP: 0010:[802a679f] [802a679f] d_splice_alias+0x1f/0x100
RSP: :810061543b08 EFLAGS: 00010282
RAX: RBX: fffb RCX: 880d3454
RDX: 810048542750 RSI: 81005e114e10 RDI: fffb
with inode equal to (struct inode *)-5. Which is ERR_PTR(-EIO)...
[880d2395] :isofs:isofs_lookup+0x395/0x4a0
inode = NULL;
if (found) {
inode = isofs_iget(dir-i_sb, block, offset);
if (!inode) {
unlock_kernel();
return ERR_PTR(-EACCES);
}
}
unlock_kernel();
return d_splice_alias(inode, dentry);
So we've got ERR_PTR(-EIO) from isofs_iget(). Bloody odd, seeing that
isofs_iget() either explicitly returns NULL or does
if (inode (inode-i_state I_NEW)) {
sb-s_op-read_inode(inode);
unlock_new_inode(inode);
}
return inode;
which would not manage to return ERR_PTR(-EIO), no matter what - it would
die on access to inode-i_state. I don't have -mm tree at hand, check if
there's anything affected in these areas. Perhaps somebody tried to pass
error values from isofs_iget() and forgot to update callers?
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: isofs oops - d_splice_alias+0x1f (2.6.24-rc5-mm1)
* Pekka J Enberg [EMAIL PROTECTED] wrote:
return ERR_PTR(-EACCES);
}
+ if (is_bad_inode(inode)) {
+ unlock_kernel();
+ iput(inode);
+ return ERR_PTR(-ENOENT);
+ }
fs/isofs/rock.c:474 parse_rock_ridge_inode_internal() seems buggy too:
reloc =
isofs_iget(inode-i_sb,
ISOFS_I(inode)-i_first_extent,
0);
if (!reloc)
goto out;
it should probably do !reloc || is_bad_inode(inode) as well.
and there are about 5 other callsites as well that only check for a NULL
return.
perhaps the better fix would be to add this to inode.c:isofs_iget():
if (inode (inode-i_state I_NEW)) {
sb-s_op-read_inode(inode);
unlock_new_inode(inode);
if (s_bad_inode(inode))
inode = NULL;
}
?
Ingo
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: isofs oops - d_splice_alias+0x1f (2.6.24-rc5-mm1)
On Thu, Jan 03, 2008 at 03:51:28PM +0200, Pekka J Enberg wrote: Does the following patch fix it? Highly doubtful, and doesn't deal with the real problem, AFAICS... -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: isofs oops - d_splice_alias+0x1f (2.6.24-rc5-mm1)
On Thu, Jan 03, 2008 at 03:11:20PM +0100, Ingo Molnar wrote: and there are about 5 other callsites as well that only check for a NULL return. ... except that this is *not* getting NULL or make_bad_inode(); that's getting ERR_PTR(), which can't happen in mainline. So that looks like an -mm patch changing calling conventions and not updating callers... -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: isofs oops - d_splice_alias+0x1f (2.6.24-rc5-mm1)
On 01/03/2008 02:51 PM, Pekka J Enberg wrote:
Hi Jiri,
On Thu, 3 Jan 2008, Jiri Slaby wrote:
this happened, while playing with broken dvd.
[snip]
Buffer I/O error on device sr0, logical block 5441
end_request: I/O error, dev sr0, sector 136
ISOFS: unable to read i-node block
Unable to handle kernel NULL pointer dereference at 00ad RIP:
[802a679f] d_splice_alias+0x1f/0x100
[snip]
Call Trace:
[880d2395] :isofs:isofs_lookup+0x395/0x4a0
[802a565b] d_alloc+0x2b/0x1d0
[8029a97c] do_lookup+0x1ac/0x200
Does the following patch fix it?
Pekka
[PATCH] isofs: check for bad inode in isofs_lookup
From: Pekka Enberg [EMAIL PROTECTED]
If isofs_read_inode() fails to read one of the inode blocks from disk, it
returns a bad inode.
Signed-off-by: Pekka Enberg [EMAIL PROTECTED]
---
fs/isofs/namei.c |5 +
1 file changed, 5 insertions(+)
Index: linux-2.6/fs/isofs/namei.c
===
--- linux-2.6.orig/fs/isofs/namei.c
+++ linux-2.6/fs/isofs/namei.c
@@ -183,6 +183,11 @@ struct dentry *isofs_lookup(struct inode
unlock_kernel();
return ERR_PTR(-EACCES);
}
+ if (is_bad_inode(inode)) {
+ unlock_kernel();
+ iput(inode);
+ return ERR_PTR(-ENOENT);
+ }
}
unlock_kernel();
return d_splice_alias(inode, dentry);
Can't say, the DVD seems to be OK, I don't know what was wrong (as I can say,
this happened several times in the past yet and after reboot everything OK; I
suspect gnome auto mounter -- multiple machines, several DVD ROMs, same disk
with OS, similar errors, but that's too few to report).
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
