[Cluster-devel] [PATCH] gfs2: fix slab corruption during mounting and umounting gfs file system

[Cluster-devel] [PATCH] gfs2: fix slab corruption during mounting and umounting gfs file system

[Thomas Tai]

When using cman-3.0.12.1 and gfs2-utils-3.0.12.1, mounting and
unmounting GFS2 file system would cause kernel to hang. The slab
allocator suggests that it is likely a double free memory corrruption.
The issue is traced back to v3.9-rc6 where a patch is submitted to
use kzalloc() for storing a bitmap instead of using a local variable.
The intention is to allocate memory during mounting and to free memory
during unmounting. The original patch misses a code path which has
already freed the memory and caused memory corruption. This patch sets
the memory pointer to NULL after the memory is freed, so that double
free memory corruption will not be happened.

gdlm_mount()
  '-- set_recover_size() which use kzalloc()
  '-- if dlm does not support ops callbacks then
          '--- free_recover_size() which use kfree()

gldm_unmount()
  '-- free_recover_size() which use kfree()

Previous patch which introduce the double free issue is
commit 57c7310b8eb9 ("GFS2: use kmalloc for lvb bitmap")

Signed-off-by: Thomas Tai <thomas.tai@oracle.com>
Reviewed-by: Liam R. Howlett <Liam.Howlett@Oracle.com>
---
 fs/gfs2/lock_dlm.c |    1 +
 1 files changed, 1 insertions(+), 0 deletions(-)

diff --git a/fs/gfs2/lock_dlm.c b/fs/gfs2/lock_dlm.c
index 0515f0a..1d98b8a 100644
--- a/fs/gfs2/lock_dlm.c
+++ b/fs/gfs2/lock_dlm.c
@@ -1059,6 +1059,7 @@ static void free_recover_size(struct lm_lockstruct *ls)
 	ls->ls_recover_submit = NULL;
 	ls->ls_recover_result = NULL;
 	ls->ls_recover_size = 0;
+	ls->ls_lvb_bits = NULL;
 }
 
 /* dlm calls before it does lock recovery */
-- 
1.7.1

[Cluster-devel] [PATCH] gfs2: fix slab corruption during mounting and umounting gfs file system

[Bob Peterson]

----- Original Message -----
| When using cman-3.0.12.1 and gfs2-utils-3.0.12.1, mounting and
| unmounting GFS2 file system would cause kernel to hang. The slab
| allocator suggests that it is likely a double free memory corrruption.
| The issue is traced back to v3.9-rc6 where a patch is submitted to
| use kzalloc() for storing a bitmap instead of using a local variable.
| The intention is to allocate memory during mounting and to free memory
| during unmounting. The original patch misses a code path which has
| already freed the memory and caused memory corruption. This patch sets
| the memory pointer to NULL after the memory is freed, so that double
| free memory corruption will not be happened.
| 
| gdlm_mount()
|   '-- set_recover_size() which use kzalloc()
|   '-- if dlm does not support ops callbacks then
|           '--- free_recover_size() which use kfree()
| 
| gldm_unmount()
|   '-- free_recover_size() which use kfree()
| 
| Previous patch which introduce the double free issue is
| commit 57c7310b8eb9 ("GFS2: use kmalloc for lvb bitmap")
| 
| Signed-off-by: Thomas Tai <thomas.tai@oracle.com>
| Reviewed-by: Liam R. Howlett <Liam.Howlett@Oracle.com>
| ---
Hi,

Thanks. This is now pushed to the for-next branch of the linux-gfs2 tree:
https://git.kernel.org/pub/scm/linux/kernel/git/gfs2/linux-gfs2.git/commit/fs/gfs2?h=for-next&id=cc1dfa8b7571ea16dec9a29e0f4c4cad90b2a761

Please note that I fixed up the grammar a bit in your patch description.

Regards,

Bob Peterson
Red Hat File Systems

[Cluster-devel] [PATCH] gfs2: fix slab corruption during mounting and umounting gfs file system

[Thomas Tai]

On 2017-08-15 01:10 PM, Bob Peterson wrote:
> ----- Original Message -----
> | When using cman-3.0.12.1 and gfs2-utils-3.0.12.1, mounting and
> | unmounting GFS2 file system would cause kernel to hang. The slab
> | allocator suggests that it is likely a double free memory corrruption.
> | The issue is traced back to v3.9-rc6 where a patch is submitted to
> | use kzalloc() for storing a bitmap instead of using a local variable.
> | The intention is to allocate memory during mounting and to free memory
> | during unmounting. The original patch misses a code path which has
> | already freed the memory and caused memory corruption. This patch sets
> | the memory pointer to NULL after the memory is freed, so that double
> | free memory corruption will not be happened.
> |
> | gdlm_mount()
> |   '-- set_recover_size() which use kzalloc()
> |   '-- if dlm does not support ops callbacks then
> |           '--- free_recover_size() which use kfree()
> |
> | gldm_unmount()
> |   '-- free_recover_size() which use kfree()
> |
> | Previous patch which introduce the double free issue is
> | commit 57c7310b8eb9 ("GFS2: use kmalloc for lvb bitmap")
> |
> | Signed-off-by: Thomas Tai <thomas.tai@oracle.com>
> | Reviewed-by: Liam R. Howlett <Liam.Howlett@Oracle.com>
> | ---
> Hi,
> 
> Thanks. This is now pushed to the for-next branch of the linux-gfs2 tree:
> https://git.kernel.org/pub/scm/linux/kernel/git/gfs2/linux-gfs2.git/commit/fs/gfs2?h=for-next&id=cc1dfa8b7571ea16dec9a29e0f4c4cad90b2a761
> 
> Please note that I fixed up the grammar a bit in your patch description.
> 

Hi Bob,
Thank you very much for your quick response and helps.

Thanks,
Thomas


> Regards,
> 
> Bob Peterson
> Red Hat File Systems
>