2013-10-29 02:00:09

by Wei Yongjun

Subject: [PATCH] befs: fix return value check in befs_iget()

From: Wei Yongjun <[email protected]>

In case of error, the function iget_locked() returns NULL pointer
not ERR_PTR(). The IS_ERR() test in the return value check should
be replaced with NULL test.

Signed-off-by: Wei Yongjun <[email protected]>
---
fs/befs/linuxvfs.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/fs/befs/linuxvfs.c b/fs/befs/linuxvfs.c
index e9c75e2..d714dda 100644
--- a/fs/befs/linuxvfs.c
+++ b/fs/befs/linuxvfs.c
@@ -319,8 +319,8 @@ static struct inode *befs_iget(struct super_block *sb, unsigned long ino)
befs_debug(sb, "---> befs_read_inode() " "inode = %lu", ino);

inode = iget_locked(sb, ino);
- if (IS_ERR(inode))
- return inode;
+ if (!inode)
+ return ERR_PTR(-ENOMEM);
if (!(inode->i_state & I_NEW))
return inode;

2013-10-29 15:09:52

by Serge E. Hallyn

Subject: Re: [PATCH] befs: fix return value check in befs_iget()

Quoting Wei Yongjun ([email protected]):
> From: Wei Yongjun <[email protected]>
>
> In case of error, the function iget_locked() returns NULL pointer
> not ERR_PTR(). The IS_ERR() test in the return value check should
> be replaced with NULL test.
>
> Signed-off-by: Wei Yongjun <[email protected]>

Acked-by: Serge Hallyn <[email protected]>

> ---
> fs/befs/linuxvfs.c | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/fs/befs/linuxvfs.c b/fs/befs/linuxvfs.c
> index e9c75e2..d714dda 100644
> --- a/fs/befs/linuxvfs.c
> +++ b/fs/befs/linuxvfs.c
> @@ -319,8 +319,8 @@ static struct inode *befs_iget(struct super_block *sb, unsigned long ino)
> befs_debug(sb, "---> befs_read_inode() " "inode = %lu", ino);
>
> inode = iget_locked(sb, ino);
> - if (IS_ERR(inode))
> - return inode;
> + if (!inode)
> + return ERR_PTR(-ENOMEM);
> if (!(inode->i_state & I_NEW))
> return inode;
>
>
> --
> 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/

2013-10-30 17:52:40

by Kees Cook

Subject: Re: [PATCH] befs: fix return value check in befs_iget()

On Mon, Oct 28, 2013 at 7:00 PM, Wei Yongjun <[email protected]> wrote:
> From: Wei Yongjun <[email protected]>
>
> In case of error, the function iget_locked() returns NULL pointer
> not ERR_PTR(). The IS_ERR() test in the return value check should
> be replaced with NULL test.
>
> Signed-off-by: Wei Yongjun <[email protected]>

Good catch, thanks!

Acked-by: Kees Cook <[email protected]>

As an aside, Dan, how hard would this kind of mismatch be to detect with smatch?

-Kees

> ---
> fs/befs/linuxvfs.c | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/fs/befs/linuxvfs.c b/fs/befs/linuxvfs.c
> index e9c75e2..d714dda 100644
> --- a/fs/befs/linuxvfs.c
> +++ b/fs/befs/linuxvfs.c
> @@ -319,8 +319,8 @@ static struct inode *befs_iget(struct super_block *sb, unsigned long ino)
> befs_debug(sb, "---> befs_read_inode() " "inode = %lu", ino);
>
> inode = iget_locked(sb, ino);
> - if (IS_ERR(inode))
> - return inode;
> + if (!inode)
> + return ERR_PTR(-ENOMEM);
> if (!(inode->i_state & I_NEW))
> return inode;
>
>



--
Kees Cook
Chrome OS Security

2013-10-30 18:29:33

by Dan Carpenter

Subject: Re: [PATCH] befs: fix return value check in befs_iget()

On Wed, Oct 30, 2013 at 10:52:38AM -0700, Kees Cook wrote:
> On Mon, Oct 28, 2013 at 7:00 PM, Wei Yongjun <[email protected]> wrote:
> > From: Wei Yongjun <[email protected]>
> >
> > In case of error, the function iget_locked() returns NULL pointer
> > not ERR_PTR(). The IS_ERR() test in the return value check should
> > be replaced with NULL test.
> >
> > Signed-off-by: Wei Yongjun <[email protected]>
>
> Good catch, thanks!
>
> Acked-by: Kees Cook <[email protected]>
>
> As an aside, Dan, how hard would this kind of mismatch be to detect
> with smatch?

It already does, but you need the cross function database set up.

regards,
dan carpenter