* [Cluster-devel] [PATCH] GFS2: Fix potential NULL dereference in gfs2_alloc_inode
@ 2015-03-02 16:15 Andrew Price
2015-03-02 16:17 ` Steven Whitehouse
0 siblings, 1 reply; 6+ messages in thread
From: Andrew Price @ 2015-03-02 16:15 UTC (permalink / raw)
To: cluster-devel.redhat.com
Return NULL when ip is NULL instead of dereferencing it.
Signed-off-by: Andrew Price <anprice@redhat.com>
---
fs/gfs2/super.c | 13 +++++++------
1 file changed, 7 insertions(+), 6 deletions(-)
diff --git a/fs/gfs2/super.c b/fs/gfs2/super.c
index 1666382..37c59ee 100644
--- a/fs/gfs2/super.c
+++ b/fs/gfs2/super.c
@@ -1628,12 +1628,13 @@ static struct inode *gfs2_alloc_inode(struct super_block *sb)
struct gfs2_inode *ip;
ip = kmem_cache_alloc(gfs2_inode_cachep, GFP_KERNEL);
- if (ip) {
- ip->i_flags = 0;
- ip->i_gl = NULL;
- ip->i_rgd = NULL;
- ip->i_res = NULL;
- }
+ if (!ip)
+ return NULL;
+
+ ip->i_flags = 0;
+ ip->i_gl = NULL;
+ ip->i_rgd = NULL;
+ ip->i_res = NULL;
return &ip->i_inode;
}
--
1.9.3
^ permalink raw reply related [flat|nested] 6+ messages in thread* [Cluster-devel] [PATCH] GFS2: Fix potential NULL dereference in gfs2_alloc_inode
2015-03-02 16:15 [Cluster-devel] [PATCH] GFS2: Fix potential NULL dereference in gfs2_alloc_inode Andrew Price
@ 2015-03-02 16:17 ` Steven Whitehouse
2015-03-02 16:30 ` Andrew Price
0 siblings, 1 reply; 6+ messages in thread
From: Steven Whitehouse @ 2015-03-02 16:17 UTC (permalink / raw)
To: cluster-devel.redhat.com
Hi,
On 02/03/15 16:15, Andrew Price wrote:
> Return NULL when ip is NULL instead of dereferencing it.
>
> Signed-off-by: Andrew Price <anprice@redhat.com>
> ---
> fs/gfs2/super.c | 13 +++++++------
> 1 file changed, 7 insertions(+), 6 deletions(-)
>
> diff --git a/fs/gfs2/super.c b/fs/gfs2/super.c
> index 1666382..37c59ee 100644
> --- a/fs/gfs2/super.c
> +++ b/fs/gfs2/super.c
> @@ -1628,12 +1628,13 @@ static struct inode *gfs2_alloc_inode(struct super_block *sb)
> struct gfs2_inode *ip;
>
> ip = kmem_cache_alloc(gfs2_inode_cachep, GFP_KERNEL);
> - if (ip) {
> - ip->i_flags = 0;
> - ip->i_gl = NULL;
> - ip->i_rgd = NULL;
> - ip->i_res = NULL;
> - }
> + if (!ip)
> + return NULL;
> +
> + ip->i_flags = 0;
> + ip->i_gl = NULL;
> + ip->i_rgd = NULL;
> + ip->i_res = NULL;
> return &ip->i_inode;
> }
>
I'm not sure that I see the problem here... it should just return NULL
if ip is NULL, since ip->i_inode is the first element of ip,
Steve.
^ permalink raw reply [flat|nested] 6+ messages in thread* [Cluster-devel] [PATCH] GFS2: Fix potential NULL dereference in gfs2_alloc_inode
2015-03-02 16:17 ` Steven Whitehouse
@ 2015-03-02 16:30 ` Andrew Price
2015-03-02 17:05 ` Andreas Gruenbacher
0 siblings, 1 reply; 6+ messages in thread
From: Andrew Price @ 2015-03-02 16:30 UTC (permalink / raw)
To: cluster-devel.redhat.com
On 02/03/15 16:17, Steven Whitehouse wrote:
> Hi,
>
> On 02/03/15 16:15, Andrew Price wrote:
>> Return NULL when ip is NULL instead of dereferencing it.
>>
>> Signed-off-by: Andrew Price <anprice@redhat.com>
>> ---
>> fs/gfs2/super.c | 13 +++++++------
>> 1 file changed, 7 insertions(+), 6 deletions(-)
>>
>> diff --git a/fs/gfs2/super.c b/fs/gfs2/super.c
>> index 1666382..37c59ee 100644
>> --- a/fs/gfs2/super.c
>> +++ b/fs/gfs2/super.c
>> @@ -1628,12 +1628,13 @@ static struct inode *gfs2_alloc_inode(struct
>> super_block *sb)
>> struct gfs2_inode *ip;
>> ip = kmem_cache_alloc(gfs2_inode_cachep, GFP_KERNEL);
>> - if (ip) {
>> - ip->i_flags = 0;
>> - ip->i_gl = NULL;
>> - ip->i_rgd = NULL;
>> - ip->i_res = NULL;
>> - }
>> + if (!ip)
>> + return NULL;
>> +
>> + ip->i_flags = 0;
>> + ip->i_gl = NULL;
>> + ip->i_rgd = NULL;
>> + ip->i_res = NULL;
>> return &ip->i_inode;
>> }
>
> I'm not sure that I see the problem here... it should just return NULL
> if ip is NULL, since ip->i_inode is the first element of ip,
Ah, so it is. Self-NACK then.
Andy
^ permalink raw reply [flat|nested] 6+ messages in thread* [Cluster-devel] [PATCH] GFS2: Fix potential NULL dereference in gfs2_alloc_inode
2015-03-02 16:30 ` Andrew Price
@ 2015-03-02 17:05 ` Andreas Gruenbacher
2015-03-02 17:31 ` [Cluster-devel] [PATCH] GFS2: Improve readability of gfs2_alloc_inode Andrew Price
0 siblings, 1 reply; 6+ messages in thread
From: Andreas Gruenbacher @ 2015-03-02 17:05 UTC (permalink / raw)
To: cluster-devel.redhat.com
On 03/02/2015 05:30 PM, Andrew Price wrote:
> On 02/03/15 16:17, Steven Whitehouse wrote:
>> Hi,
>>
>> On 02/03/15 16:15, Andrew Price wrote:
> Ah, so it is. Self-NACK then.
The patch itself is still worth it though, it's not self-evident ip and
&ip->i_inode are the same.
Andreas
^ permalink raw reply [flat|nested] 6+ messages in thread
* [Cluster-devel] [PATCH] GFS2: Improve readability of gfs2_alloc_inode
2015-03-02 17:05 ` Andreas Gruenbacher
@ 2015-03-02 17:31 ` Andrew Price
2015-03-03 14:57 ` Bob Peterson
0 siblings, 1 reply; 6+ messages in thread
From: Andrew Price @ 2015-03-02 17:31 UTC (permalink / raw)
To: cluster-devel.redhat.com
Returning &ip->i_inode when ip is NULL is safe as i_inode is the first
member in struct gfs2_inode, but that's not immediately obvious.
Reorganize gfs2_alloc_inode to avoid any doubt.
Signed-off-by: Andrew Price <anprice@redhat.com>
---
Re-sending with a more appropriate commit log based on Andreas' comments.
fs/gfs2/super.c | 13 +++++++------
1 file changed, 7 insertions(+), 6 deletions(-)
diff --git a/fs/gfs2/super.c b/fs/gfs2/super.c
index 1666382..37c59ee 100644
--- a/fs/gfs2/super.c
+++ b/fs/gfs2/super.c
@@ -1628,12 +1628,13 @@ static struct inode *gfs2_alloc_inode(struct super_block *sb)
struct gfs2_inode *ip;
ip = kmem_cache_alloc(gfs2_inode_cachep, GFP_KERNEL);
- if (ip) {
- ip->i_flags = 0;
- ip->i_gl = NULL;
- ip->i_rgd = NULL;
- ip->i_res = NULL;
- }
+ if (!ip)
+ return NULL;
+
+ ip->i_flags = 0;
+ ip->i_gl = NULL;
+ ip->i_rgd = NULL;
+ ip->i_res = NULL;
return &ip->i_inode;
}
--
1.9.3
^ permalink raw reply related [flat|nested] 6+ messages in thread* [Cluster-devel] [PATCH] GFS2: Improve readability of gfs2_alloc_inode
2015-03-02 17:31 ` [Cluster-devel] [PATCH] GFS2: Improve readability of gfs2_alloc_inode Andrew Price
@ 2015-03-03 14:57 ` Bob Peterson
0 siblings, 0 replies; 6+ messages in thread
From: Bob Peterson @ 2015-03-03 14:57 UTC (permalink / raw)
To: cluster-devel.redhat.com
----- Original Message -----
> Returning &ip->i_inode when ip is NULL is safe as i_inode is the first
> member in struct gfs2_inode, but that's not immediately obvious.
> Reorganize gfs2_alloc_inode to avoid any doubt.
>
> Signed-off-by: Andrew Price <anprice@redhat.com>
> ---
>
> Re-sending with a more appropriate commit log based on Andreas' comments.
>
> fs/gfs2/super.c | 13 +++++++------
> 1 file changed, 7 insertions(+), 6 deletions(-)
>
> diff --git a/fs/gfs2/super.c b/fs/gfs2/super.c
> index 1666382..37c59ee 100644
> --- a/fs/gfs2/super.c
> +++ b/fs/gfs2/super.c
> @@ -1628,12 +1628,13 @@ static struct inode *gfs2_alloc_inode(struct
> super_block *sb)
> struct gfs2_inode *ip;
>
> ip = kmem_cache_alloc(gfs2_inode_cachep, GFP_KERNEL);
> - if (ip) {
> - ip->i_flags = 0;
> - ip->i_gl = NULL;
> - ip->i_rgd = NULL;
> - ip->i_res = NULL;
> - }
> + if (!ip)
> + return NULL;
> +
> + ip->i_flags = 0;
> + ip->i_gl = NULL;
> + ip->i_rgd = NULL;
> + ip->i_res = NULL;
> return &ip->i_inode;
> }
>
> --
> 1.9.3
ACK,
Bob Peterson
Red Hat File Systems
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2015-03-03 14:57 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2015-03-02 16:15 [Cluster-devel] [PATCH] GFS2: Fix potential NULL dereference in gfs2_alloc_inode Andrew Price
2015-03-02 16:17 ` Steven Whitehouse
2015-03-02 16:30 ` Andrew Price
2015-03-02 17:05 ` Andreas Gruenbacher
2015-03-02 17:31 ` [Cluster-devel] [PATCH] GFS2: Improve readability of gfs2_alloc_inode Andrew Price
2015-03-03 14:57 ` Bob Peterson
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).