2013-10-22 07:38:35

by Denis Efremov

Subject: [PATCH] xfs:xfs_dir2_node.c: pointer use before check for null

Reorder of assert and args pointer dereference.

Found by Linux Driver Verification project (linuxtesting.org) -
PVS-Studio analyzer.

Signed-off-by: Denis Efremov <[email protected]>
---
fs/xfs/xfs_dir2_node.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/fs/xfs/xfs_dir2_node.c b/fs/xfs/xfs_dir2_node.c
index 4c3dba7..0ba7382 100644
--- a/fs/xfs/xfs_dir2_node.c
+++ b/fs/xfs/xfs_dir2_node.c
@@ -1365,8 +1365,8 @@ xfs_dir2_leafn_split(
* Allocate space for a new leaf node.
*/
args = state->args;
- mp = args->dp->i_mount;
ASSERT(args != NULL);
+ mp = args->dp->i_mount;
ASSERT(oldblk->magic == XFS_DIR2_LEAFN_MAGIC);
error = xfs_da_grow_inode(args, &blkno);
if (error) {
--
1.8.3.1

2013-10-22 20:33:30

by Dave Chinner

Subject: Re: [PATCH] xfs:xfs_dir2_node.c: pointer use before check for null

On Tue, Oct 22, 2013 at 11:36:15AM +0400, Denis Efremov wrote:
> Reorder of assert and args pointer dereference.
>
> Found by Linux Driver Verification project (linuxtesting.org) -
> PVS-Studio analyzer.
>
> Signed-off-by: Denis Efremov <[email protected]>
> ---
> fs/xfs/xfs_dir2_node.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/fs/xfs/xfs_dir2_node.c b/fs/xfs/xfs_dir2_node.c
> index 4c3dba7..0ba7382 100644
> --- a/fs/xfs/xfs_dir2_node.c
> +++ b/fs/xfs/xfs_dir2_node.c
> @@ -1365,8 +1365,8 @@ xfs_dir2_leafn_split(
> * Allocate space for a new leaf node.
> */
> args = state->args;
> - mp = args->dp->i_mount;
> ASSERT(args != NULL);
> + mp = args->dp->i_mount;

Just remove the ASSERT. Either way we are going to panic.

Cheers,

Dave.
--
Dave Chinner
[email protected]

2013-10-25 11:55:38

by Denis Efremov

Subject: [PATCH v2] xfs:xfs_dir2_node.c: pointer use before check for null

ASSERT on args takes place after args dereference.
This assertion is redundant since we are going to panic anyway.

Found by Linux Driver Verification project (linuxtesting.org) -
PVS-Studio analyzer.

Signed-off-by: Denis Efremov <[email protected]>
---
fs/xfs/xfs_dir2_node.c | 1 -
1 file changed, 1 deletion(-)

diff --git a/fs/xfs/xfs_dir2_node.c b/fs/xfs/xfs_dir2_node.c
index 4c3dba7..dc814df 100644
--- a/fs/xfs/xfs_dir2_node.c
+++ b/fs/xfs/xfs_dir2_node.c
@@ -1366,7 +1366,6 @@ xfs_dir2_leafn_split(
*/
args = state->args;
mp = args->dp->i_mount;
- ASSERT(args != NULL);
ASSERT(oldblk->magic == XFS_DIR2_LEAFN_MAGIC);
error = xfs_da_grow_inode(args, &blkno);
if (error) {
--
1.8.3.1

2013-10-25 15:06:16

by Ben Myers

Subject: Re: [PATCH v2] xfs:xfs_dir2_node.c: pointer use before check for null

On Fri, Oct 25, 2013 at 03:53:25PM +0400, Denis Efremov wrote:
> ASSERT on args takes place after args dereference.
> This assertion is redundant since we are going to panic anyway.
>
> Found by Linux Driver Verification project (linuxtesting.org) -
> PVS-Studio analyzer.
>
> Signed-off-by: Denis Efremov <[email protected]>

Looks good.

Reviewed-by: Ben Myers <[email protected]>

2013-10-31 15:56:07

by Ben Myers

Subject: Re: [PATCH v2] xfs:xfs_dir2_node.c: pointer use before check for null

On Fri, Oct 25, 2013 at 10:06:09AM -0500, Ben Myers wrote:
> On Fri, Oct 25, 2013 at 03:53:25PM +0400, Denis Efremov wrote:
> > ASSERT on args takes place after args dereference.
> > This assertion is redundant since we are going to panic anyway.
> >
> > Found by Linux Driver Verification project (linuxtesting.org) -
> > PVS-Studio analyzer.
> >
> > Signed-off-by: Denis Efremov <[email protected]>
>
> Looks good.
>
> Reviewed-by: Ben Myers <[email protected]>

Applied. Thanks Denis.