On Sat, Jun 03, 2006 at 11:34:39AM -0400, Florin Malita wrote:
> 'bh_result' & 'inode' are explicitly checked against NULL so they
> shouldn't be dereferenced prior to that.
>
> Coverity ID: 1273, 1274.
> --- a/fs/ocfs2/aops.c
> +++ b/fs/ocfs2/aops.c
> @@ -559,13 +559,14 @@ static int ocfs2_direct_IO_get_blocks(st
> u64 p_blkno;
> int contig_blocks;
> unsigned char blocksize_bits;
> - unsigned long max_blocks = bh_result->b_size >> inode->i_blkbits;
> + unsigned long max_blocks;
>
> if (!inode || !bh_result) {
> mlog(ML_ERROR, "inode or bh_result is null\n");
> return -EIO;
> }
>
> + max_blocks = bh_result->b_size >> inode->i_blkbits;
> blocksize_bits = inode->i_sb->s_blocksize_bits;
AFAICS, the patch is BS, as usual with this type of patches.
Can "inode" and "bh_result" be NULL here? I bet they can't.
1. The sole purpose of ocfs2_direct_IO_get_blocks() is to be given to
blockdev_direct_IO_no_locking() at fs/ocfs2/aops.c:662
Can blockdev_direct_IO_no_locking() call it with first or third
arg being NULL?
2. blockdev_direct_IO_no_locking() is static inline wrapper
include/linux/fs.h:1687
Can __blockdev_direct_IO() call it's "get_block" arg with first
or third arg being NULL?
3. Aiee, __blockdev_direct_IO() is normal function (fs/direct-io.c:1179)
and it gives damn "get_block" arg to direct_io_worker() at
fs/direct-io.c:1276
4. direct_io_worker() initializes ->get_block method of "dio" structure
and then does something my tiny mind fails to understand. All I can
see it doesn't call nor change method later.
Where is ->get_block called?
5. That what grep(1) is for. One place
fs/direct-io.c:553: ret = (*dio->get_block)(dio->inode, fs_startblk,
fs/direct-io.c-554- map_bh, create);
Now time to press PageUp. We're seeking for dereferences of
"dio->inode" and "map_bh" above.
6. Both are immediately out of question:
536 map_bh->b_size = fs_count << dio->inode->i_blkbits;
^^^^^^ ^^^^^^^^^^
so it can't be NULL ditto
537
538 create = dio->rw == WRITE;
539 if (dio->lock_type == DIO_LOCKING) {
540 if (dio->block_in_file < (i_size_read(dio->inode) >>
541 dio->blkbits))
542 create = 0;
543 } else if (dio->lock_type == DIO_NO_LOCKING) {
544 create = 0;
545 }
546
547 /*
548 * For writes inside i_size we forbid block creations: only
549 * overwrites are permitted. We fall back to buffered writes
550 * at a higher level for inside-i_size block-instantiating
551 * writes.
552 */
553 ret = (*dio->get_block)(dio->inode, fs_startblk,
^^^^^^^^^^^^^^ ^^^^^^^^^^
offending call offending arg
554 map_bh, create);
^^^^^^
offending arg
-
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/
[Index of Archives]
[Kernel Newbies]
[Netfilter]
[Bugtraq]
[Photo]
[Stuff]
[Gimp]
[Yosemite News]
[MIPS Linux]
[ARM Linux]
[Linux Security]
[Linux RAID]
[Video 4 Linux]
[Linux for the blind]
[Linux Resources]
