thr3ads.net - Ocfs2 devel - [Ocfs2-devel] [PATCH 1/6] ocfs2/cluster: Fix possible null pointer dereference [Nov 2011]

If this information is useful, please help other people find it:
Share via:

Sunil Mushran

2011-Nov-18 00:32 UTC

[Ocfs2-devel] [PATCH 1/6] ocfs2/cluster: Fix possible null pointer dereference

Patch fixes some possible null pointer dereferences that were detected by the
static code analyser, smatch.

Reported-by: Dan Carpenter <error27 at gmail.com>
Signed-off-by: Sunil Mushran <sunil.mushran at oracle.com>
---
 fs/ocfs2/cluster/tcp.c |   10 +++++-----
 1 files changed, 5 insertions(+), 5 deletions(-)

diff --git a/fs/ocfs2/cluster/tcp.c b/fs/ocfs2/cluster/tcp.c
index 044e7b5..bf9494d 100644
--- a/fs/ocfs2/cluster/tcp.c
+++ b/fs/ocfs2/cluster/tcp.c
@@ -546,8 +546,9 @@ static void o2net_set_nn_state(struct o2net_node *nn,
 	}
 
 	if (was_valid && !valid) {
-		printk(KERN_NOTICE "o2net: No longer connected to "
-		       SC_NODEF_FMT "\n", SC_NODEF_ARGS(old_sc));
+		if (old_sc)
+			printk(KERN_NOTICE "o2net: No longer connected to "
+			       SC_NODEF_FMT "\n", SC_NODEF_ARGS(old_sc));
 		o2net_complete_nodes_nsw(nn);
 	}
 
@@ -1700,13 +1701,12 @@ static void o2net_start_connect(struct work_struct
*work)
 		ret = 0;
 
 out:
-	if (ret) {
+	if (ret && sc) {
 		printk(KERN_NOTICE "o2net: Connect attempt to " SC_NODEF_FMT
 		       " failed with errno %d\n", SC_NODEF_ARGS(sc), ret);
 		/* 0 err so that another will be queued and attempted
 		 * from set_nn_state */
-		if (sc)
-			o2net_ensure_shutdown(nn, sc, 0);
+		o2net_ensure_shutdown(nn, sc, 0);
 	}
 	if (sc)
 		sc_put(sc);
-- 
1.7.5.4

Sunil Mushran

2011-Nov-18 00:32 UTC

head link

[Ocfs2-devel] [PATCH 2/6] ocfs2: Add missing copyright in few files

Signed-off-by: Sunil Mushran <sunil.mushran at oracle.com>
---
 fs/ocfs2/mmap.h         |   18 ++++++++++++++++++
 fs/ocfs2/ocfs2_trace.h  |   19 +++++++++++++++++++
 fs/ocfs2/quota.h        |   16 ++++++++++++++--
 fs/ocfs2/quota_global.c |   20 ++++++++++++++++++--
 fs/ocfs2/quota_local.c  |   19 +++++++++++++++++--
 5 files changed, 86 insertions(+), 6 deletions(-)

diff --git a/fs/ocfs2/mmap.h b/fs/ocfs2/mmap.h
index 1274ee0..63ca43e 100644
--- a/fs/ocfs2/mmap.h
+++ b/fs/ocfs2/mmap.h
@@ -1,3 +1,21 @@
+/* -*- mode: c; c-basic-offset: 8; -*-
+ * vim: noexpandtab sw=8 ts=8 sts=0:
+ *
+ * mmap.h
+ *
+ * Copyright (C) 2004, 2011 Oracle.  All rights reserved.
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU General Public
+ * License version 2 as published by the Free Software Foundation.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+ * General Public License for more details.
+ * Copyright (C) 2004, 2011 Oracle.  All rights reserved.
+ */
+
 #ifndef OCFS2_MMAP_H
 #define OCFS2_MMAP_H
 
diff --git a/fs/ocfs2/ocfs2_trace.h b/fs/ocfs2/ocfs2_trace.h
index 3b481f4..6f3f5b4 100644
--- a/fs/ocfs2/ocfs2_trace.h
+++ b/fs/ocfs2/ocfs2_trace.h
@@ -1,3 +1,22 @@
+/* -*- mode: c; c-basic-offset: 8; -*-
+ * vim: noexpandtab sw=8 ts=8 sts=0:
+ *
+ * ocfs2_trace.h
+ *
+ * Trace event macros
+ *
+ * Copyright (C) 2010, 2011 Oracle.  All rights reserved.
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU General Public
+ * License version 2 as published by the Free Software Foundation.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+ * General Public License for more details.
+ */
+
 #undef TRACE_SYSTEM
 #define TRACE_SYSTEM ocfs2
 
diff --git a/fs/ocfs2/quota.h b/fs/ocfs2/quota.h
index d5ab56c..36ef1e2 100644
--- a/fs/ocfs2/quota.h
+++ b/fs/ocfs2/quota.h
@@ -1,9 +1,21 @@
-/*
- * quota.h for OCFS2
+/* -*- mode: c; c-basic-offset: 8; -*-
+ * vim: noexpandtab sw=8 ts=8 sts=0:
+ *
+ * quota.h
  *
  * On disk quota structures for local and global quota file, in-memory
  * structures.
  *
+ * Copyright (C) 2008, 2011 Novell.  All rights reserved.
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU General Public
+ * License version 2 as published by the Free Software Foundation.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+ * General Public License for more details.
  */
 
 #ifndef _OCFS2_QUOTA_H
diff --git a/fs/ocfs2/quota_global.c b/fs/ocfs2/quota_global.c
index 92fcd57..b24eab3 100644
--- a/fs/ocfs2/quota_global.c
+++ b/fs/ocfs2/quota_global.c
@@ -1,6 +1,22 @@
-/*
- *  Implementation of operations over global quota file
+/* -*- mode: c; c-basic-offset: 8; -*-
+ * vim: noexpandtab sw=8 ts=8 sts=0:
+ *
+ * quota_global.c
+ *
+ * Implementation of operations over global quota file
+ *
+ * Copyright (C) 2008, 2011 Novell.  All rights reserved.
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU General Public
+ * License version 2 as published by the Free Software Foundation.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+ * General Public License for more details.
  */
+
 #include <linux/spinlock.h>
 #include <linux/fs.h>
 #include <linux/slab.h>
diff --git a/fs/ocfs2/quota_local.c b/fs/ocfs2/quota_local.c
index 942fd65..62da63a 100644
--- a/fs/ocfs2/quota_local.c
+++ b/fs/ocfs2/quota_local.c
@@ -1,5 +1,20 @@
-/*
- *  Implementation of operations over local quota file
+/* -*- mode: c; c-basic-offset: 8; -*-
+ * vim: noexpandtab sw=8 ts=8 sts=0:
+ *
+ * quota_local.c
+ *
+ * Implementation of operations over local quota file
+ *
+ * Copyright (C) 2008, 2011 Novell.  All rights reserved.
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU General Public
+ * License version 2 as published by the Free Software Foundation.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+ * General Public License for more details.
  */
 
 #include <linux/fs.h>
-- 
1.7.5.4

Sunil Mushran

2011-Nov-18 00:32 UTC

head link

[Ocfs2-devel] [PATCH 3/6] ocfs2: Silence message in ocfs2_global_read_info()

Mainline commit c1e8d35ef5ffb393b94a192034b5e3541e005d75 removed mlog_exit()
and replaced it with mlog_errno() in some cases.

This change in ocfs2_global_read_info() caused it to spew the following during
mount.
[   38.745584] (mount.ocfs2,3315,4):ocfs2_global_read_info:403 ERROR: status =
24

This patch silences this message.

Signed-off-by: Sunil Mushran <sunil.mushran at oracle.com>
---
 fs/ocfs2/quota_global.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/fs/ocfs2/quota_global.c b/fs/ocfs2/quota_global.c
index b24eab3..2a3f12c 100644
--- a/fs/ocfs2/quota_global.c
+++ b/fs/ocfs2/quota_global.c
@@ -413,7 +413,7 @@ int ocfs2_global_read_info(struct super_block *sb, int type)
 	INIT_DELAYED_WORK(&oinfo->dqi_sync_work, qsync_work_fn);
 	schedule_delayed_work(&oinfo->dqi_sync_work,
 			      msecs_to_jiffies(oinfo->dqi_syncms));
-
+	status = 0;
 out_err:
 	if (status)
 		mlog_errno(status);
-- 
1.7.5.4

Sunil Mushran

2011-Nov-18 00:33 UTC

head link

[Ocfs2-devel] [PATCH 4/6] ocfs2/dlm: Use track_lock when manipulating tracking_list

Commit b0d4f817ba5de8adb875ace594554a96d7737710 introduced dlm->track_lock
to protect operations on dlm->tracking_list. But it was still using the
older lock (dlm->spin_lock) to add new resources to the list.

Signed-off-by: Sunil Mushran <sunil.mushran at oracle.com>
---
 fs/ocfs2/dlm/dlmmaster.c |    4 ++--
 1 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/fs/ocfs2/dlm/dlmmaster.c b/fs/ocfs2/dlm/dlmmaster.c
index 005261c..347d588 100644
--- a/fs/ocfs2/dlm/dlmmaster.c
+++ b/fs/ocfs2/dlm/dlmmaster.c
@@ -597,9 +597,9 @@ static void dlm_init_lockres(struct dlm_ctxt *dlm,
 
 	res->last_used = 0;
 
-	spin_lock(&dlm->spinlock);
+	spin_lock(&dlm->track_lock);
 	list_add_tail(&res->tracking, &dlm->tracking_list);
-	spin_unlock(&dlm->spinlock);
+	spin_unlock(&dlm->track_lock);
 
 	memset(res->lvb, 0, DLM_LVB_LEN);
 	memset(res->refmap, 0, sizeof(res->refmap));
-- 
1.7.5.4

Sunil Mushran

2011-Nov-18 00:33 UTC

head link

[Ocfs2-devel] [PATCH 5/6] ocfs2/dlm: Fix list traversal in dlm_process_recovery_data

This issue, detected via static analysis, was introduced by commit 800deef3
that replaced the existing list_for_each() with list_for_each_entry(). We
have no record of it hitting users during runtime. This could be because
this lock list is supposed to have only one lock that is owned by that node
averting the case in which lock is set to NULL.

Reported-by: Julia Lawall <julia at diku.dk>
Signed-off-by: Sunil Mushran <sunil.mushran at oracle.com>
---
 fs/ocfs2/dlm/dlmrecovery.c |   10 +++++-----
 1 files changed, 5 insertions(+), 5 deletions(-)

diff --git a/fs/ocfs2/dlm/dlmrecovery.c b/fs/ocfs2/dlm/dlmrecovery.c
index 01ebfd0..c881be6 100644
--- a/fs/ocfs2/dlm/dlmrecovery.c
+++ b/fs/ocfs2/dlm/dlmrecovery.c
@@ -1752,7 +1752,7 @@ static int dlm_process_recovery_data(struct dlm_ctxt *dlm,
 				     struct dlm_migratable_lockres *mres)
 {
 	struct dlm_migratable_lock *ml;
-	struct list_head *queue;
+	struct list_head *queue, *iter;
 	struct list_head *tmpq = NULL;
 	struct dlm_lock *newlock = NULL;
 	struct dlm_lockstatus *lksb = NULL;
@@ -1796,11 +1796,11 @@ static int dlm_process_recovery_data(struct dlm_ctxt
*dlm,
 			spin_lock(&res->spinlock);
 			for (j = DLM_GRANTED_LIST; j <= DLM_BLOCKED_LIST; j++) {
 				tmpq = dlm_list_idx_to_ptr(res, j);
-				list_for_each_entry(lock, tmpq, list) {
-					if (lock->ml.cookie != ml->cookie)
-						lock = NULL;
-					else
+				list_for_each(iter, tmpq) {
+					lock = list_entry(iter, struct dlm_lock, list);
+					if (lock->ml.cookie == ml->cookie)
 						break;
+					lock = NULL;
 				}
 				if (lock)
 					break;
-- 
1.7.5.4

Sunil Mushran

2011-Nov-18 00:33 UTC

head link

[Ocfs2-devel] [PATCH 6/6] ocfs2: Tighten free bit calculation in the global bitmap

When clearing bits in the global bitmap, we do not test the current bit value.
This patch tightens the code by considering the possiblity that the bit being
cleared was already cleared.

Now this should not happen. But we are seeing stray instances in which free
bit count in the global bitmap exceeds the total bit count. In each instance
the bitmap is correct. Only the free bit count is incorrect.

This patch checks the current bit value and increments the free bit count
only if the bit was previously set. It also prints information to allow
us to debug further.

Signed-off-by: Sunil Mushran <sunil.mushran at oracle.com>
---
 fs/ocfs2/suballoc.c |   26 +++++++++++++++++++++++---
 1 files changed, 23 insertions(+), 3 deletions(-)

diff --git a/fs/ocfs2/suballoc.c b/fs/ocfs2/suballoc.c
index ba5d97e..972f0b1 100644
--- a/fs/ocfs2/suballoc.c
+++ b/fs/ocfs2/suballoc.c
@@ -2386,6 +2386,7 @@ static int ocfs2_block_group_clear_bits(handle_t *handle,
 	int status;
 	unsigned int tmp;
 	struct ocfs2_group_desc *undo_bg = NULL;
+	unsigned int bits_cleared = 0;
 
 	/* The caller got this descriptor from
 	 * ocfs2_read_group_descriptor().  Any corruption is a code bug. */
@@ -2413,13 +2414,32 @@ static int ocfs2_block_group_clear_bits(handle_t
*handle,
 
 	tmp = num_bits;
 	while(tmp--) {
-		ocfs2_clear_bit((bit_off + tmp),
-				(unsigned long *) bg->bg_bitmap);
+		if (ocfs2_test_bit((bit_off + tmp),
+				   (unsigned long *) bg->bg_bitmap)) {
+			ocfs2_clear_bit((bit_off + tmp),
+					(unsigned long *) bg->bg_bitmap);
+			bits_cleared++;
+		}
 		if (undo_fn)
 			undo_fn(bit_off + tmp,
 				(unsigned long *) undo_bg->bg_bitmap);
 	}
-	le16_add_cpu(&bg->bg_free_bits_count, num_bits);
+
+	le16_add_cpu(&bg->bg_free_bits_count, bits_cleared);
+
+	/*
+	 * If encountered, it means we are clearing bits multiple times. While
+	 * we are handling the case, we still need to be alerted to its
+	 * occurrence. Hence, marking it as an ERROR and not NOTICE.
+	 */
+	if (num_bits != bits_cleared) {
+		mlog(ML_ERROR, "Trying to clear %u bits at offset %u in group "
+		     "descriptor # %llu (device %s), needed to clear %u bits\n",
+		     num_bits, bit_off,
+		     (unsigned long long)le64_to_cpu(bg->bg_blkno),
+		     alloc_inode->i_sb->s_id, bits_cleared);
+	}
+
 	if (le16_to_cpu(bg->bg_free_bits_count) > le16_to_cpu(bg->bg_bits)) {
 		ocfs2_error(alloc_inode->i_sb, "Group descriptor # %llu has bit"
 			    " count %u but claims %u are freed. num_bits %d",
-- 
1.7.5.4

Ocfs2 devel - Nov 2011 - [PATCH 1/6] ocfs2/cluster: Fix possible null pointer dereference

[Ocfs2-devel] [PATCH 1/6] ocfs2/cluster: Fix possible null pointer dereference

[Ocfs2-devel] [PATCH 2/6] ocfs2: Add missing copyright in few files

[Ocfs2-devel] [PATCH 3/6] ocfs2: Silence message in ocfs2_global_read_info()

[Ocfs2-devel] [PATCH 4/6] ocfs2/dlm: Use track_lock when manipulating tracking_list

[Ocfs2-devel] [PATCH 5/6] ocfs2/dlm: Fix list traversal in dlm_process_recovery_data

[Ocfs2-devel] [PATCH 6/6] ocfs2: Tighten free bit calculation in the global bitmap