Test for sb_getblk return value
Subsystems (2)
PATCH 1/1 Test for sb_getblk return value
Hi all,
As we discussed earlier, I'm sending a patch that adds test for the return value of sb_getblk. This time I focused on the code of the ext2/3 filesystems. I'm assuming that getblk fails happens due to I/O errors and thus returning returning an EIO back wherever it's needed.
Al, hope it's better this time.
Signature · 5 lines
===================================== Glauber de Oliveira Costa IBM Linux Technology Center - Brazil [email protected] =====================================
fs/ext2/inode.c +4 -1
@@ -439,7 +439,10 @@ static int ext2_alloc_branch(struct inod* Get buffer_head for parent block, zero it out and set * the pointer to new one, then send parent to disk. */ - bh =sb_getblk (inode->i_sb, parent); +if (!(bh =sb_getblk (inode->i_sb, parent))){ + err = -EIO ; +break ; + }lock_buffer (bh);memset (bh->b_data,0 , blocksize); branch[n].bh = bh;
fs/ext3/inode.c +7 -3
@@ -523,14 +523,15 @@ static int ext3_alloc_branch(handle_t *hif (!nr)break ; branch[n].key =cpu_to_le32 (nr); - keys = n+1 ;/* * Get buffer_head for parent block, zero it out * and set the pointer to new one, then send * parent to disk. */ - bh =sb_getblk (inode->i_sb, parent); +if (!(bh =sb_getblk (inode->i_sb, parent))) +break ; + keys = n+1 ; branch[n].bh = bh;lock_buffer (bh);BUFFER_TRACE (bh,"call get_create_access" );
@@ -863,7 +864,10 @@ struct buffer_head *ext3_getblk(handle_t *errp =ext3_get_block_handle (handle, inode, block, &dummy, create,1 );if (!*errp &&buffer_mapped (&dummy)) {struct buffer_head *bh; - bh =sb_getblk (inode->i_sb, dummy.b_blocknr); +if (!(bh =sb_getblk (inode->i_sb, dummy.b_blocknr))){ + *errp = -EIO ; +return NULL ; + }if (buffer_new (&dummy)) {J_ASSERT (create !=0 );J_ASSERT (handle !=0 );
fs/ext3/resize.c +10 -3
@@ -117,7 +117,8 @@ static struct buffer_head *bclean(handlestruct buffer_head *bh;int err; - bh =sb_getblk (sb, blk); +if (!(bh =sb_getblk (sb, blk))) +return ERR_PTR (-EIO );if ((err =ext3_journal_get_write_access (handle, bh))) {brelse (bh); bh =ERR_PTR (err);
@@ -201,7 +202,10 @@ static int setup_new_group_blocks(structext3_debug ("update backup group %#04lx (+%d)\n" , block, bit); - gdb =sb_getblk (sb, block); +if (!(gdb =sb_getblk (sb, block))){ + err = -EIO ; + goto exit_bh; + }if ((err =ext3_journal_get_write_access (handle, gdb))) {brelse (gdb); goto exit_bh;
@@ -642,7 +646,10 @@ static void update_backups(struct super_ (err = ext3_journal_restart(handle, EXT3_MAX_TRANS_DATA))) break; - bh =sb_getblk (sb, group * bpg + blk_off); +if (!(bh =sb_getblk (sb, group * bpg + blk_off))){ + err = -EIO ; + goto exit_err; + }ext3_debug ("update metadata backup %#04lx\n" , (unsigned long )bh->b_blocknr);if ((err =ext3_journal_get_write_access (handle, bh)))
fs/ext2/inode.c static int ext2_alloc_branch(struct inod · line 445
Quoted 13 lines · Glauber de Oliveira Costa
Hi all,
As we discussed earlier, I'm sending a patch that adds test for the return value of
sb_getblk. This time I focused on the code of the ext2/3 filesystems. I'm assuming that getblk fails happens due to I/O errors and thus returning returning an EIO back wherever it's needed.fs/ext2/inode.c +4
-1- bh =sb_getblk (inode->i_sb, parent); +if (!(bh =sb_getblk (inode->i_sb, parent))){ + err = -EIO ; +break ; + }
Would be more readable as
bh = sb_getblk(inode->i_sb, parent);
if (!bh) {
err = -EIO;
break;
}--
Jesper Juhl <[email protected]>
Don't top-post http://www.catb.org/~esr/jargon/html/T/top-post.html
Plain text mails only, please http://www.expita.com/nomime.html
-------------------------------------------------------
This SF.Net email is sponsored by:
Power Architecture Resource Center: Free content, downloads, discussions, and more. http://solutions.newsforge.com/ibmarch.tmpl
I'm resending it now with the changes you suggested. Actually, 2 copies of it follows.
In the first one(v2), I kept the style in the changes in resize.c, as this seems to be the default way things like this are done there. In the other, (v3), I did statement checks in the way you suggested in both files.
Also, sorry for the last mail. I got a problem with my relay, and my mail address was sent wrong before I noticed that. Mails sent to it will probably return.
Quoted 33 lines · Jesper Juhl
Review
Quoted 13 lines · Glauber de Oliveira Costa
Hi all,
As we discussed earlier, I'm sending a patch that adds test for the return value of
sb_getblk. This time I focused on the code of the ext2/3 filesystems. I'm assuming that getblk fails happens due to I/O errors and thus returning returning an EIO back wherever it's needed.(quoted diff) +4
-1- bh = sb_getblk(inode->i_sb, parent); + if (!(bh = sb_getblk(inode->i_sb, parent))){ + err = -EIO; + break; + }Would be more readable as
bh = sb_getblk(inode->i_sb, parent); if (!bh) { err = -EIO; break; }--
Jesper Juhl <[email protected]>
Don't top-post http://www.catb.org/~esr/jargon/html/T/top-post.html
Plain text mails only, please http://www.expita.com/nomime.html
-
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/
Signature · 5 lines
===================================== Glauber de Oliveira Costa IBM Linux Technology Center - Brazil [email protected] =====================================
fs/ext2/inode.c +4 -0
@@ -440,6 +440,10 @@ static int ext2_alloc_branch(struct inod* the pointer to new one, then send parent to disk. */ bh =sb_getblk (inode->i_sb, parent); +if (!bh){ + err = -EIO ; +break ; + }lock_buffer (bh);memset (bh->b_data,0 , blocksize); branch[n].bh = bh;
fs/ext3/inode.c +7 -1
@@ -523,7 +523,6 @@ static int ext3_alloc_branch(handle_t *hif (!nr)break ; branch[n].key =cpu_to_le32 (nr); - keys = n+1 ;/* * Get buffer_head for parent block, zero it out
@@ -531,6 +530,9 @@ static int ext3_alloc_branch(handle_t *h* parent to disk. */ bh =sb_getblk (inode->i_sb, parent); +if (!bh) +break ; + keys = n+1 ; branch[n].bh = bh;lock_buffer (bh);BUFFER_TRACE (bh,"call get_create_access" );
@@ -864,6 +866,10 @@ struct buffer_head *ext3_getblk(handle_tif (!*errp &&buffer_mapped (&dummy)) {struct buffer_head *bh; bh =sb_getblk (inode->i_sb, dummy.b_blocknr); +if (!bh){ + *errp = -EIO ; +return NULL ; + }if (buffer_new (&dummy)) {J_ASSERT (create !=0 );J_ASSERT (handle !=0 );
fs/ext3/resize.c +10 -3
@@ -117,7 +117,8 @@ static struct buffer_head *bclean(handlestruct buffer_head *bh;int err; - bh =sb_getblk (sb, blk); +if (!(bh =sb_getblk (sb, blk))) +return ERR_PTR (-EIO );if ((err =ext3_journal_get_write_access (handle, bh))) {brelse (bh); bh =ERR_PTR (err);
@@ -201,7 +202,10 @@ static int setup_new_group_blocks(structext3_debug ("update backup group %#04lx (+%d)\n" , block, bit); - gdb =sb_getblk (sb, block); +if (!(gdb =sb_getblk (sb, block))){ + err = -EIO ; + goto exit_bh; + }if ((err =ext3_journal_get_write_access (handle, gdb))) {brelse (gdb); goto exit_bh;
@@ -642,7 +646,10 @@ static void update_backups(struct super_ (err = ext3_journal_restart(handle, EXT3_MAX_TRANS_DATA))) break; - bh =sb_getblk (sb, group * bpg + blk_off); +if (!(bh =sb_getblk (sb, group * bpg + blk_off))){ + err = -EIO ; + goto exit_err; + }ext3_debug ("update metadata backup %#04lx\n" , (unsigned long )bh->b_blocknr);if ((err =ext3_journal_get_write_access (handle, bh)))
fs/ext2/inode.c +4 -0
@@ -440,6 +440,10 @@ static int ext2_alloc_branch(struct inod* the pointer to new one, then send parent to disk. */ bh =sb_getblk (inode->i_sb, parent); +if (!bh){ + err = -EIO ; +break ; + }lock_buffer (bh);memset (bh->b_data,0 , blocksize); branch[n].bh = bh;
fs/ext3/inode.c +7 -1
@@ -523,7 +523,6 @@ static int ext3_alloc_branch(handle_t *hif (!nr)break ; branch[n].key =cpu_to_le32 (nr); - keys = n+1 ;/* * Get buffer_head for parent block, zero it out
@@ -531,6 +530,9 @@ static int ext3_alloc_branch(handle_t *h* parent to disk. */ bh =sb_getblk (inode->i_sb, parent); +if (!bh) +break ; + keys = n+1 ; branch[n].bh = bh;lock_buffer (bh);BUFFER_TRACE (bh,"call get_create_access" );
@@ -864,6 +866,10 @@ struct buffer_head *ext3_getblk(handle_tif (!*errp &&buffer_mapped (&dummy)) {struct buffer_head *bh; bh =sb_getblk (inode->i_sb, dummy.b_blocknr); +if (!bh){ + *errp = -EIO ; +return NULL ; + }if (buffer_new (&dummy)) {J_ASSERT (create !=0 );J_ASSERT (handle !=0 );
fs/ext3/resize.c +10 -0
@@ -118,6 +118,8 @@ static struct buffer_head *bclean(handleint err; bh =sb_getblk (sb, blk); +if (!bh) +return ERR_PTR (-EIO );if ((err =ext3_journal_get_write_access (handle, bh))) {brelse (bh); bh =ERR_PTR (err);
@@ -202,6 +204,10 @@ static int setup_new_group_blocks(structext3_debug ("update backup group %#04lx (+%d)\n" , block, bit); gdb =sb_getblk (sb, block); +if (!bh){ + err = -EIO ; + goto exit_bh; + }if ((err =ext3_journal_get_write_access (handle, gdb))) {brelse (gdb); goto exit_bh;
@@ -643,6 +649,10 @@ static void update_backups(struct super_break ; bh =sb_getblk (sb, group * bpg + blk_off); +if (!bh){ + err = -EIO ; + goto exit_err; + }ext3_debug ("update metadata backup %#04lx\n" , (unsigned long )bh->b_blocknr);if ((err =ext3_journal_get_write_access (handle, bh)))
Quoted 3 lines · Glauber de Oliveira Costa
I'm resending it now with the changes you suggested. Actually, 2 copies of it follows.
argh. Please never attach multiple patches to a single email.
And please always include a complete, uptodate changelog with each iteration of a patch. I don't want to have to troll back through the mailing list, identify the initial changelog and then replay the email thread making any needed updates to that changelog.
Also please review section 11 of Documentation/SubmittingPatches then include a Signed-off-by: with your patches.
Quoted 3 lines
In the first one(v2), I kept the style in the changes in
resize.c, as this seems to be the default way things like this are done there. In the other, (v3), I did statement checks in the way you suggested in both files.
Don't worry about the surrounding style - if it's wrong, it's wrong. Just stick with Documentation/CodingStyle.
Do this:
if (!bh) {and not this:
if (!bh){Quoted 3 lines
Also, sorry for the last mail. I got a problem with my relay, and my mail address was sent wrong before I noticed that. Mails sent to it will probably return.
The change to update_backups() is wrong - it will leave a JBD transaction open on return.
Please fix all that up and resend.
http://www.zip.com.au/~akpm/linux/patches/stuff/tpp.txt may prove useful, thanks.
-------------------------------------------------------
This SF.Net email is sponsored by:
Power Architecture Resource Center: Free content, downloads, discussions, and more. http://solutions.newsforge.com/ibmarch.tmpl
Andrew,
Thanks a lot for your patience and review. And also for the document. It was really helpfull. Patch follows in next mail.
Quoted 42 lines · Andrew Morton
Quoted 3 lines · Glauber de Oliveira Costa
I'm resending it now with the changes you suggested. Actually, 2 copies of it follows.
argh. Please never attach multiple patches to a single email.
And please always include a complete, uptodate changelog with each iteration of a patch. I don't want to have to troll back through the mailing list, identify the initial changelog and then replay the email thread making any needed updates to that changelog.
Also please review section 11 of
Documentation/SubmittingPatchesthen include a Signed-off-by: with your patches.Quoted 3 lines
In the first one(v2), I kept the style in the changes in
resize.c, as this seems to be the default way things like this are done there. In the other, (v3), I did statement checks in the way you suggested in both files.Don't worry about the surrounding style - if it's wrong, it's wrong. Just stick with
Documentation/CodingStyle.Do this:
if (!bh) {and not this:
if (!bh){Quoted 3 lines
Also, sorry for the last mail. I got a problem with my relay, and my mail address was sent wrong before I noticed that. Mails sent to it will probably return.
The change to
update_backups()is wrong - it will leave a JBD transaction open on return.Please fix all that up and resend.
http://www.zip.com.au/~akpm/linux/patches/stuff/tpp.txt may prove useful, thanks.
Signature · 5 lines
===================================== Glauber de Oliveira Costa IBM Linux Technology Center - Brazil [email protected] =====================================
This patch adds tests for the return value of sb_getblk() in the ext2/3 filesystems. In fs/buffer.c it is stated that the getblk() function never fails. However, it does can return NULL in some situations due to I/O errors, which may lead us to NULL pointer dereferences
Signature · 5 lines
===================================== Glauber de Oliveira Costa IBM Linux Technology Center - Brazil [email protected] =====================================
fs/ext2/inode.c +4 -0
@@ -440,6 +440,10 @@ static int ext2_alloc_branch(struct inod* the pointer to new one, then send parent to disk. */ bh =sb_getblk (inode->i_sb, parent); +if (!bh) { + err = -EIO ; +break ; + }lock_buffer (bh);memset (bh->b_data,0 , blocksize); branch[n].bh = bh;
fs/ext3/inode.c +7 -1
@@ -523,7 +523,6 @@ static int ext3_alloc_branch(handle_t *hif (!nr)break ; branch[n].key =cpu_to_le32 (nr); - keys = n+1 ;/* * Get buffer_head for parent block, zero it out
@@ -531,6 +530,9 @@ static int ext3_alloc_branch(handle_t *h* parent to disk. */ bh =sb_getblk (inode->i_sb, parent); +if (!bh) +break ; + keys = n+1 ; branch[n].bh = bh;lock_buffer (bh);BUFFER_TRACE (bh,"call get_create_access" );
@@ -864,6 +866,10 @@ struct buffer_head *ext3_getblk(handle_tif (!*errp &&buffer_mapped (&dummy)) {struct buffer_head *bh; bh =sb_getblk (inode->i_sb, dummy.b_blocknr); +if (!bh) { + *errp = -EIO ; +return NULL ; + }if (buffer_new (&dummy)) {J_ASSERT (create !=0 );J_ASSERT (handle !=0 );
fs/ext3/resize.c +10 -0
@@ -118,6 +118,8 @@ static struct buffer_head *bclean(handleint err; bh =sb_getblk (sb, blk); +if (!bh) +return ERR_PTR (-EIO );if ((err =ext3_journal_get_write_access (handle, bh))) {brelse (bh); bh =ERR_PTR (err);
@@ -202,6 +204,10 @@ static int setup_new_group_blocks(structext3_debug ("update backup group %#04lx (+%d)\n" , block, bit); gdb =sb_getblk (sb, block); +if (!bh) { + err = -EIO ; + goto exit_bh; + }if ((err =ext3_journal_get_write_access (handle, gdb))) {brelse (gdb); goto exit_bh;
@@ -643,6 +649,10 @@ static void update_backups(struct super_break ; bh =sb_getblk (sb, group * bpg + blk_off); +if (!bh) { + err = -EIO ; +break ; + }ext3_debug ("update metadata backup %#04lx\n" , (unsigned long )bh->b_blocknr);if ((err =ext3_journal_get_write_access (handle, bh)))