* [Cluster-devel] [GFS2 PATCH] GFS2: attempt to access beyond end of device creating file
[not found] <0964abc6-3d78-4cdd-bce8-9e8466886b0b@zmail12.collab.prod.int.phx2.redhat.com>
@ 2012-03-13 17:55 ` Bob Peterson
2012-03-13 21:01 ` Steven Whitehouse
0 siblings, 1 reply; 5+ messages in thread
From: Bob Peterson @ 2012-03-13 17:55 UTC (permalink / raw)
To: cluster-devel.redhat.com
Hi,
This patch initializes the hash table cache when inodes are
created in order to prevent using a hash table pointer left over
from a previous inode. It also adds boundary checking on the
hash table.
Regards,
Bob Peterson
Red Hat File Systems
Signed-off-by: Bob Peterson <rpeterso@redhat.com>
--
diff --git a/fs/gfs2/dir.c b/fs/gfs2/dir.c
index c35573a..c2eb20f 100644
--- a/fs/gfs2/dir.c
+++ b/fs/gfs2/dir.c
@@ -740,6 +740,7 @@ static int get_leaf_nr(struct gfs2_inode *dip, u32 index,
hash = gfs2_dir_get_hash_table(dip);
if (IS_ERR(hash))
return PTR_ERR(hash);
+ BUG_ON(index >= (1 << dip->i_depth));
*leaf_out = be64_to_cpu(*(hash + index));
return 0;
}
diff --git a/fs/gfs2/super.c b/fs/gfs2/super.c
index 6172fa7..bd2abe3 100644
--- a/fs/gfs2/super.c
+++ b/fs/gfs2/super.c
@@ -1576,6 +1576,7 @@ static struct inode *gfs2_alloc_inode(struct super_block *sb)
ip->i_flags = 0;
ip->i_gl = NULL;
ip->i_rgd = NULL;
+ ip->i_hash_cache = NULL;
}
return &ip->i_inode;
}
^ permalink raw reply related [flat|nested] 5+ messages in thread
* [Cluster-devel] [GFS2 PATCH] GFS2: attempt to access beyond end of device creating file
2012-03-13 17:55 ` [Cluster-devel] [GFS2 PATCH] GFS2: attempt to access beyond end of device creating file Bob Peterson
@ 2012-03-13 21:01 ` Steven Whitehouse
2012-03-14 12:41 ` Bob Peterson
0 siblings, 1 reply; 5+ messages in thread
From: Steven Whitehouse @ 2012-03-13 21:01 UTC (permalink / raw)
To: cluster-devel.redhat.com
Hi,
On Tue, 2012-03-13 at 13:55 -0400, Bob Peterson wrote:
> Hi,
>
> This patch initializes the hash table cache when inodes are
> created in order to prevent using a hash table pointer left over
> from a previous inode. It also adds boundary checking on the
> hash table.
>
> Regards,
>
> Bob Peterson
> Red Hat File Systems
>
> Signed-off-by: Bob Peterson <rpeterso@redhat.com>
> --
> diff --git a/fs/gfs2/dir.c b/fs/gfs2/dir.c
> index c35573a..c2eb20f 100644
> --- a/fs/gfs2/dir.c
> +++ b/fs/gfs2/dir.c
> @@ -740,6 +740,7 @@ static int get_leaf_nr(struct gfs2_inode *dip, u32 index,
> hash = gfs2_dir_get_hash_table(dip);
> if (IS_ERR(hash))
> return PTR_ERR(hash);
> + BUG_ON(index >= (1 << dip->i_depth));
> *leaf_out = be64_to_cpu(*(hash + index));
> return 0;
> }
> diff --git a/fs/gfs2/super.c b/fs/gfs2/super.c
> index 6172fa7..bd2abe3 100644
> --- a/fs/gfs2/super.c
> +++ b/fs/gfs2/super.c
> @@ -1576,6 +1576,7 @@ static struct inode *gfs2_alloc_inode(struct super_block *sb)
> ip->i_flags = 0;
> ip->i_gl = NULL;
> ip->i_rgd = NULL;
> + ip->i_hash_cache = NULL;
> }
> return &ip->i_inode;
> }
>
The question is how does this pointer land up being not NULL to start
with... I thought that you were looking into that from your earlier
comment in the bug,
Steve.
^ permalink raw reply [flat|nested] 5+ messages in thread
* [Cluster-devel] [GFS2 PATCH] GFS2: attempt to access beyond end of device creating file
2012-03-13 21:01 ` Steven Whitehouse
@ 2012-03-14 12:41 ` Bob Peterson
2012-03-14 12:58 ` Steven Whitehouse
2012-03-15 12:19 ` Bob Peterson
0 siblings, 2 replies; 5+ messages in thread
From: Bob Peterson @ 2012-03-14 12:41 UTC (permalink / raw)
To: cluster-devel.redhat.com
----- Original Message -----
| Hi,
|
| On Tue, 2012-03-13 at 13:55 -0400, Bob Peterson wrote:
| > Hi,
| >
| > This patch initializes the hash table cache when inodes are
| > created in order to prevent using a hash table pointer left over
| > from a previous inode. It also adds boundary checking on the
| > hash table.
| >
| > Regards,
| >
| > Bob Peterson
| > Red Hat File Systems
| >
| > Signed-off-by: Bob Peterson <rpeterso@redhat.com>
| > --
| > diff --git a/fs/gfs2/dir.c b/fs/gfs2/dir.c
| > index c35573a..c2eb20f 100644
| > --- a/fs/gfs2/dir.c
| > +++ b/fs/gfs2/dir.c
| > @@ -740,6 +740,7 @@ static int get_leaf_nr(struct gfs2_inode *dip,
| > u32 index,
| > hash = gfs2_dir_get_hash_table(dip);
| > if (IS_ERR(hash))
| > return PTR_ERR(hash);
| > + BUG_ON(index >= (1 << dip->i_depth));
| > *leaf_out = be64_to_cpu(*(hash + index));
| > return 0;
| > }
| > diff --git a/fs/gfs2/super.c b/fs/gfs2/super.c
| > index 6172fa7..bd2abe3 100644
| > --- a/fs/gfs2/super.c
| > +++ b/fs/gfs2/super.c
| > @@ -1576,6 +1576,7 @@ static struct inode *gfs2_alloc_inode(struct
| > super_block *sb)
| > ip->i_flags = 0;
| > ip->i_gl = NULL;
| > ip->i_rgd = NULL;
| > + ip->i_hash_cache = NULL;
| > }
| > return &ip->i_inode;
| > }
| >
|
| The question is how does this pointer land up being not NULL to start
| with... I thought that you were looking into that from your earlier
| comment in the bug,
|
| Steve.
Hi,
My belief is that the i_hash_cache pointer was non-NULL because of
a code path in the RHEL6 kernel that does not exist in the upstream
kernel. (RHEL6 function gfs2_clear_inode was not invalidating the hash
table cache like it should, but it doesn't exist in upstream GFS2).
If my suspicion is correct, setting the i_hash_cache pointer to NULL
from gfs2_alloc_inode is unnecessary and purely precautionary.
I'm running some tests on the RHEL6 kernel to help prove that theory,
but the test will probably take all day (it's a long-running test).
I can't reproduce the original problem with the upstream kernel
because I can't run that test on my upstream box (it requires a
special RHEL6 environment set up by a third party).
Regards,
Bob Peterson
Red Hat File Systems
^ permalink raw reply [flat|nested] 5+ messages in thread
* [Cluster-devel] [GFS2 PATCH] GFS2: attempt to access beyond end of device creating file
2012-03-14 12:41 ` Bob Peterson
@ 2012-03-14 12:58 ` Steven Whitehouse
2012-03-15 12:19 ` Bob Peterson
1 sibling, 0 replies; 5+ messages in thread
From: Steven Whitehouse @ 2012-03-14 12:58 UTC (permalink / raw)
To: cluster-devel.redhat.com
Hi,
On Wed, 2012-03-14 at 08:41 -0400, Bob Peterson wrote:
> ----- Original Message -----
> | Hi,
> |
> | On Tue, 2012-03-13 at 13:55 -0400, Bob Peterson wrote:
> | > Hi,
> | >
> | > This patch initializes the hash table cache when inodes are
> | > created in order to prevent using a hash table pointer left over
> | > from a previous inode. It also adds boundary checking on the
> | > hash table.
> | >
> | > Regards,
> | >
> | > Bob Peterson
> | > Red Hat File Systems
> | >
> | > Signed-off-by: Bob Peterson <rpeterso@redhat.com>
> | > --
> | > diff --git a/fs/gfs2/dir.c b/fs/gfs2/dir.c
> | > index c35573a..c2eb20f 100644
> | > --- a/fs/gfs2/dir.c
> | > +++ b/fs/gfs2/dir.c
> | > @@ -740,6 +740,7 @@ static int get_leaf_nr(struct gfs2_inode *dip,
> | > u32 index,
> | > hash = gfs2_dir_get_hash_table(dip);
> | > if (IS_ERR(hash))
> | > return PTR_ERR(hash);
> | > + BUG_ON(index >= (1 << dip->i_depth));
> | > *leaf_out = be64_to_cpu(*(hash + index));
> | > return 0;
> | > }
> | > diff --git a/fs/gfs2/super.c b/fs/gfs2/super.c
> | > index 6172fa7..bd2abe3 100644
> | > --- a/fs/gfs2/super.c
> | > +++ b/fs/gfs2/super.c
> | > @@ -1576,6 +1576,7 @@ static struct inode *gfs2_alloc_inode(struct
> | > super_block *sb)
> | > ip->i_flags = 0;
> | > ip->i_gl = NULL;
> | > ip->i_rgd = NULL;
> | > + ip->i_hash_cache = NULL;
> | > }
> | > return &ip->i_inode;
> | > }
> | >
> |
> | The question is how does this pointer land up being not NULL to start
> | with... I thought that you were looking into that from your earlier
> | comment in the bug,
> |
> | Steve.
> Hi,
>
> My belief is that the i_hash_cache pointer was non-NULL because of
> a code path in the RHEL6 kernel that does not exist in the upstream
> kernel. (RHEL6 function gfs2_clear_inode was not invalidating the hash
> table cache like it should, but it doesn't exist in upstream GFS2).
>
> If my suspicion is correct, setting the i_hash_cache pointer to NULL
> from gfs2_alloc_inode is unnecessary and purely precautionary.
> I'm running some tests on the RHEL6 kernel to help prove that theory,
> but the test will probably take all day (it's a long-running test).
>
It would be better to add a test, if you are going to add something
here. We don't want to hide any future problem and there is no valid
reason for the pointer to be non-NULL at this point,
Steve.
> I can't reproduce the original problem with the upstream kernel
> because I can't run that test on my upstream box (it requires a
> special RHEL6 environment set up by a third party).
>
> Regards,
>
> Bob Peterson
> Red Hat File Systems
^ permalink raw reply [flat|nested] 5+ messages in thread
* [Cluster-devel] [GFS2 PATCH] GFS2: attempt to access beyond end of device creating file
2012-03-14 12:41 ` Bob Peterson
2012-03-14 12:58 ` Steven Whitehouse
@ 2012-03-15 12:19 ` Bob Peterson
1 sibling, 0 replies; 5+ messages in thread
From: Bob Peterson @ 2012-03-15 12:19 UTC (permalink / raw)
To: cluster-devel.redhat.com
----- Original Message -----
| | The question is how does this pointer land up being not NULL to
| | start
| | with... I thought that you were looking into that from your earlier
| | comment in the bug,
| |
| | Steve.
| Hi,
|
| My belief is that the i_hash_cache pointer was non-NULL because of
| a code path in the RHEL6 kernel that does not exist in the upstream
| kernel. (RHEL6 function gfs2_clear_inode was not invalidating the
| hash
| table cache like it should, but it doesn't exist in upstream GFS2).
|
| If my suspicion is correct, setting the i_hash_cache pointer to NULL
| from gfs2_alloc_inode is unnecessary and purely precautionary.
| I'm running some tests on the RHEL6 kernel to help prove that theory,
| but the test will probably take all day (it's a long-running test).
|
| I can't reproduce the original problem with the upstream kernel
| because I can't run that test on my upstream box (it requires a
| special RHEL6 environment set up by a third party).
|
| Regards,
|
| Bob Peterson
| Red Hat File Systems
|
Hi,
I confirmed my suspicion above. The non-NULL pointer was caused by
bug in a code path that does not apply to the upstream kernel.
Therefore, I self-NAK and withdraw this patch.
Regards,
Bob Peterson
Red Hat File Systems
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2012-03-15 12:19 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
[not found] <0964abc6-3d78-4cdd-bce8-9e8466886b0b@zmail12.collab.prod.int.phx2.redhat.com>
2012-03-13 17:55 ` [Cluster-devel] [GFS2 PATCH] GFS2: attempt to access beyond end of device creating file Bob Peterson
2012-03-13 21:01 ` Steven Whitehouse
2012-03-14 12:41 ` Bob Peterson
2012-03-14 12:58 ` Steven Whitehouse
2012-03-15 12:19 ` Bob Peterson
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.