2015-05-21 10:28:57

by Maninder Singh

Subject: [EDT] [PATCH] devpts/inode.c : Fix Possible dentry NULL dereference

EP-F6AA0618C49C4AEDA73BFF1B39950BAB

Hi,

Subject: [PATCH 1/1] devpts/inode.c : Fix Possible dentry NULL dereference

Issue reported by static tool Analyzer (Prevent).
d_find_alias can return NULL to deentry, Thus we need NULL check
before calling d_delete(dentry)

Signed-off-by: Maninder Singh <[email protected]>
Reviewed-by: Vaneet Narang <[email protected]>
---
fs/devpts/inode.c | 3 ++-
1 files changed, 2 insertions(+), 1 deletions(-)

diff --git a/fs/devpts/inode.c b/fs/devpts/inode.c
index add5663..0350ac2 100644
--- a/fs/devpts/inode.c
+++ b/fs/devpts/inode.c
@@ -663,7 +663,8 @@ void devpts_pty_kill(struct inode *inode)
dentry = d_find_alias(inode);

drop_nlink(inode);
- d_delete(dentry);
+ if (dentry)
+ d_delete(dentry);
dput(dentry); /* d_alloc_name() in devpts_pty_new() */
dput(dentry); /* d_find_alias above */

--
1.7.1

Thanks ,
Maninder Singh????{.n?+???????+%?????ݶ??w??{.n?+????{??G?????{ay?ʇڙ?,j??f???h?????????z_??(?階?ݢj"???m??????G????????????&???~???iO???z??v?^?m????????????I?

2015-05-21 10:42:06

by Richard Weinberger

Subject: Re: [EDT] [PATCH] devpts/inode.c : Fix Possible dentry NULL dereference

On Thu, May 21, 2015 at 12:28 PM, Maninder Singh
<[email protected]> wrote:
>
> EP-F6AA0618C49C4AEDA73BFF1B39950BAB

What is this?

> Hi,
>
> Subject: [PATCH 1/1] devpts/inode.c : Fix Possible dentry NULL dereference
>
> Issue reported by static tool Analyzer (Prevent).
> d_find_alias can return NULL to deentry, Thus we need NULL check
> before calling d_delete(dentry)
>
> Signed-off-by: Maninder Singh <[email protected]>
> Reviewed-by: Vaneet Narang <[email protected]>
> ---
> fs/devpts/inode.c | 3 ++-
> 1 files changed, 2 insertions(+), 1 deletions(-)
>
> diff --git a/fs/devpts/inode.c b/fs/devpts/inode.c
> index add5663..0350ac2 100644
> --- a/fs/devpts/inode.c
> +++ b/fs/devpts/inode.c
> @@ -663,7 +663,8 @@ void devpts_pty_kill(struct inode *inode)
> dentry = d_find_alias(inode);
>
> drop_nlink(inode);
> - d_delete(dentry);
> + if (dentry)
> + d_delete(dentry);
> dput(dentry); /* d_alloc_name() in devpts_pty_new() */
> dput(dentry); /* d_find_alias above */

While it is correct that d_find_alias() may return NULL, can this also
happen here?
I mean if that happens in the kill path we might have bigger trouble...

Al?

--
Thanks,
//richard

2015-05-21 11:12:46

by Al Viro

Subject: Re: [EDT] [PATCH] devpts/inode.c : Fix Possible dentry NULL dereference

On Thu, May 21, 2015 at 10:28:52AM +0000, Maninder Singh wrote:

> Subject: [PATCH 1/1] devpts/inode.c : Fix Possible dentry NULL dereference
>
> Issue reported by static tool Analyzer (Prevent).
> d_find_alias can return NULL to deentry, Thus we need NULL check
> before calling d_delete(dentry)

No, we do not. That inode must have come from devpts_pty_new() and it
wouldn't have made it out of there without having a dentry attached to
it (and pinned).

NAK.