ocfs2: don't put and assigning null to bh allocated outside
ocfs2_read_blocks() and ocfs2_read_blocks_sync() are both used to read several blocks from disk. Currently, the input argument *bhs* can be NULL or NOT. It depends on the caller's behavior. If the function fails in reading blocks from disk, the corresponding bh will be assigned to NULL and put. Obviously, above process for non-NULL input bh is not appropriate. Because the caller doesn't even know its bhs are put and re-assigned. If buffer head is managed by caller, ocfs2_read_blocks and ocfs2_read_blocks_sync() should not evaluate it to NULL. It will cause caller accessing illegal memory, thus crash. Link: http://lkml.kernel.org/r/HK2PR06MB045285E0F4FBB561F9F2F9B3D5680@HK2PR06MB0452.apcprd06.prod.outlook.com Signed-off-by: Changwei Ge <ge.changwei@h3c.com> Reviewed-by: Guozhonghua <guozhonghua@h3c.com> Cc: Mark Fasheh <mark@fasheh.com> Cc: Joel Becker <jlbec@evilplan.org> Cc: Junxiao Bi <junxiao.bi@oracle.com> Cc: Joseph Qi <jiangqi903@gmail.com> Cc: Changwei Ge <ge.changwei@h3c.com> Signed-off-by: Andrew Morton <akpm@linux-foundation.org> Signed-off-by: Linus Torvalds <torvalds@linux-foundation.org>
This commit is contained in:
parent
29aa30167a
commit
cf76c78595
|
@ -99,25 +99,34 @@ out:
|
||||||
return ret;
|
return ret;
|
||||||
}
|
}
|
||||||
|
|
||||||
|
/* Caller must provide a bhs[] with all NULL or non-NULL entries, so it
|
||||||
|
* will be easier to handle read failure.
|
||||||
|
*/
|
||||||
int ocfs2_read_blocks_sync(struct ocfs2_super *osb, u64 block,
|
int ocfs2_read_blocks_sync(struct ocfs2_super *osb, u64 block,
|
||||||
unsigned int nr, struct buffer_head *bhs[])
|
unsigned int nr, struct buffer_head *bhs[])
|
||||||
{
|
{
|
||||||
int status = 0;
|
int status = 0;
|
||||||
unsigned int i;
|
unsigned int i;
|
||||||
struct buffer_head *bh;
|
struct buffer_head *bh;
|
||||||
|
int new_bh = 0;
|
||||||
|
|
||||||
trace_ocfs2_read_blocks_sync((unsigned long long)block, nr);
|
trace_ocfs2_read_blocks_sync((unsigned long long)block, nr);
|
||||||
|
|
||||||
if (!nr)
|
if (!nr)
|
||||||
goto bail;
|
goto bail;
|
||||||
|
|
||||||
|
/* Don't put buffer head and re-assign it to NULL if it is allocated
|
||||||
|
* outside since the caller can't be aware of this alternation!
|
||||||
|
*/
|
||||||
|
new_bh = (bhs[0] == NULL);
|
||||||
|
|
||||||
for (i = 0 ; i < nr ; i++) {
|
for (i = 0 ; i < nr ; i++) {
|
||||||
if (bhs[i] == NULL) {
|
if (bhs[i] == NULL) {
|
||||||
bhs[i] = sb_getblk(osb->sb, block++);
|
bhs[i] = sb_getblk(osb->sb, block++);
|
||||||
if (bhs[i] == NULL) {
|
if (bhs[i] == NULL) {
|
||||||
status = -ENOMEM;
|
status = -ENOMEM;
|
||||||
mlog_errno(status);
|
mlog_errno(status);
|
||||||
goto bail;
|
break;
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
bh = bhs[i];
|
bh = bhs[i];
|
||||||
|
@ -158,9 +167,26 @@ int ocfs2_read_blocks_sync(struct ocfs2_super *osb, u64 block,
|
||||||
submit_bh(REQ_OP_READ, 0, bh);
|
submit_bh(REQ_OP_READ, 0, bh);
|
||||||
}
|
}
|
||||||
|
|
||||||
|
read_failure:
|
||||||
for (i = nr; i > 0; i--) {
|
for (i = nr; i > 0; i--) {
|
||||||
bh = bhs[i - 1];
|
bh = bhs[i - 1];
|
||||||
|
|
||||||
|
if (unlikely(status)) {
|
||||||
|
if (new_bh && bh) {
|
||||||
|
/* If middle bh fails, let previous bh
|
||||||
|
* finish its read and then put it to
|
||||||
|
* aovoid bh leak
|
||||||
|
*/
|
||||||
|
if (!buffer_jbd(bh))
|
||||||
|
wait_on_buffer(bh);
|
||||||
|
put_bh(bh);
|
||||||
|
bhs[i - 1] = NULL;
|
||||||
|
} else if (bh && buffer_uptodate(bh)) {
|
||||||
|
clear_buffer_uptodate(bh);
|
||||||
|
}
|
||||||
|
continue;
|
||||||
|
}
|
||||||
|
|
||||||
/* No need to wait on the buffer if it's managed by JBD. */
|
/* No need to wait on the buffer if it's managed by JBD. */
|
||||||
if (!buffer_jbd(bh))
|
if (!buffer_jbd(bh))
|
||||||
wait_on_buffer(bh);
|
wait_on_buffer(bh);
|
||||||
|
@ -170,8 +196,7 @@ int ocfs2_read_blocks_sync(struct ocfs2_super *osb, u64 block,
|
||||||
* so we can safely record this and loop back
|
* so we can safely record this and loop back
|
||||||
* to cleanup the other buffers. */
|
* to cleanup the other buffers. */
|
||||||
status = -EIO;
|
status = -EIO;
|
||||||
put_bh(bh);
|
goto read_failure;
|
||||||
bhs[i - 1] = NULL;
|
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
|
@ -179,6 +204,9 @@ bail:
|
||||||
return status;
|
return status;
|
||||||
}
|
}
|
||||||
|
|
||||||
|
/* Caller must provide a bhs[] with all NULL or non-NULL entries, so it
|
||||||
|
* will be easier to handle read failure.
|
||||||
|
*/
|
||||||
int ocfs2_read_blocks(struct ocfs2_caching_info *ci, u64 block, int nr,
|
int ocfs2_read_blocks(struct ocfs2_caching_info *ci, u64 block, int nr,
|
||||||
struct buffer_head *bhs[], int flags,
|
struct buffer_head *bhs[], int flags,
|
||||||
int (*validate)(struct super_block *sb,
|
int (*validate)(struct super_block *sb,
|
||||||
|
@ -188,6 +216,7 @@ int ocfs2_read_blocks(struct ocfs2_caching_info *ci, u64 block, int nr,
|
||||||
int i, ignore_cache = 0;
|
int i, ignore_cache = 0;
|
||||||
struct buffer_head *bh;
|
struct buffer_head *bh;
|
||||||
struct super_block *sb = ocfs2_metadata_cache_get_super(ci);
|
struct super_block *sb = ocfs2_metadata_cache_get_super(ci);
|
||||||
|
int new_bh = 0;
|
||||||
|
|
||||||
trace_ocfs2_read_blocks_begin(ci, (unsigned long long)block, nr, flags);
|
trace_ocfs2_read_blocks_begin(ci, (unsigned long long)block, nr, flags);
|
||||||
|
|
||||||
|
@ -213,6 +242,11 @@ int ocfs2_read_blocks(struct ocfs2_caching_info *ci, u64 block, int nr,
|
||||||
goto bail;
|
goto bail;
|
||||||
}
|
}
|
||||||
|
|
||||||
|
/* Don't put buffer head and re-assign it to NULL if it is allocated
|
||||||
|
* outside since the caller can't be aware of this alternation!
|
||||||
|
*/
|
||||||
|
new_bh = (bhs[0] == NULL);
|
||||||
|
|
||||||
ocfs2_metadata_cache_io_lock(ci);
|
ocfs2_metadata_cache_io_lock(ci);
|
||||||
for (i = 0 ; i < nr ; i++) {
|
for (i = 0 ; i < nr ; i++) {
|
||||||
if (bhs[i] == NULL) {
|
if (bhs[i] == NULL) {
|
||||||
|
@ -221,7 +255,8 @@ int ocfs2_read_blocks(struct ocfs2_caching_info *ci, u64 block, int nr,
|
||||||
ocfs2_metadata_cache_io_unlock(ci);
|
ocfs2_metadata_cache_io_unlock(ci);
|
||||||
status = -ENOMEM;
|
status = -ENOMEM;
|
||||||
mlog_errno(status);
|
mlog_errno(status);
|
||||||
goto bail;
|
/* Don't forget to put previous bh! */
|
||||||
|
break;
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
bh = bhs[i];
|
bh = bhs[i];
|
||||||
|
@ -316,16 +351,27 @@ int ocfs2_read_blocks(struct ocfs2_caching_info *ci, u64 block, int nr,
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
status = 0;
|
read_failure:
|
||||||
|
|
||||||
for (i = (nr - 1); i >= 0; i--) {
|
for (i = (nr - 1); i >= 0; i--) {
|
||||||
bh = bhs[i];
|
bh = bhs[i];
|
||||||
|
|
||||||
if (!(flags & OCFS2_BH_READAHEAD)) {
|
if (!(flags & OCFS2_BH_READAHEAD)) {
|
||||||
if (status) {
|
if (unlikely(status)) {
|
||||||
/* Clear the rest of the buffers on error */
|
/* Clear the buffers on error including those
|
||||||
put_bh(bh);
|
* ever succeeded in reading
|
||||||
bhs[i] = NULL;
|
*/
|
||||||
|
if (new_bh && bh) {
|
||||||
|
/* If middle bh fails, let previous bh
|
||||||
|
* finish its read and then put it to
|
||||||
|
* aovoid bh leak
|
||||||
|
*/
|
||||||
|
if (!buffer_jbd(bh))
|
||||||
|
wait_on_buffer(bh);
|
||||||
|
put_bh(bh);
|
||||||
|
bhs[i] = NULL;
|
||||||
|
} else if (bh && buffer_uptodate(bh)) {
|
||||||
|
clear_buffer_uptodate(bh);
|
||||||
|
}
|
||||||
continue;
|
continue;
|
||||||
}
|
}
|
||||||
/* We know this can't have changed as we hold the
|
/* We know this can't have changed as we hold the
|
||||||
|
@ -343,9 +389,7 @@ int ocfs2_read_blocks(struct ocfs2_caching_info *ci, u64 block, int nr,
|
||||||
* uptodate. */
|
* uptodate. */
|
||||||
status = -EIO;
|
status = -EIO;
|
||||||
clear_buffer_needs_validate(bh);
|
clear_buffer_needs_validate(bh);
|
||||||
put_bh(bh);
|
goto read_failure;
|
||||||
bhs[i] = NULL;
|
|
||||||
continue;
|
|
||||||
}
|
}
|
||||||
|
|
||||||
if (buffer_needs_validate(bh)) {
|
if (buffer_needs_validate(bh)) {
|
||||||
|
@ -355,11 +399,8 @@ int ocfs2_read_blocks(struct ocfs2_caching_info *ci, u64 block, int nr,
|
||||||
BUG_ON(buffer_jbd(bh));
|
BUG_ON(buffer_jbd(bh));
|
||||||
clear_buffer_needs_validate(bh);
|
clear_buffer_needs_validate(bh);
|
||||||
status = validate(sb, bh);
|
status = validate(sb, bh);
|
||||||
if (status) {
|
if (status)
|
||||||
put_bh(bh);
|
goto read_failure;
|
||||||
bhs[i] = NULL;
|
|
||||||
continue;
|
|
||||||
}
|
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
|
|
Loading…
Reference in New Issue