linux-kernel - [PATCH] DLM: fix a couple of races

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite for Android: free password hash cracker in your pocket

[<prev] [next>] [thread-next>] [day] [month] [year] [list]

Date:	Fri, 4 May 2007 21:49:45 +0530 (IST)
From:	Satyam Sharma <ssatyam@....iitk.ac.in>
To:	linux-kernel@...r.kernel.org
cc:	teigland@...hat.com, linux-cluster@...hat.com,
	joel.becker@...cle.com
Subject: [PATCH] DLM: fix a couple of races

Hi,

There are the following two trivially-fixed races in fs/dlm/config.c:

1. The configfs subsystem semaphore must be held by the caller when 
calling config_group_find_obj(). It's needed to walk the subsystem 
hierarchy without racing with a simultaneous mkdir(2) or rmdir(2). I 
looked around to see if there was some other way we were avoiding this 
race, but couldn't find any.

2. get_comm() does hold the subsystem semaphore but lets go too soon -- 
before grabbing a reference on the found config_item. A concurrent 
rmdir(2) could come and release the comm after the up() but before the 
config_item_get().

Patch that fixes both these bugs below.

Cheers,
S

PS: For some reason, configfs still uses a struct semaphore (as a binary 
semaphore) for configfs_subsystem.su_sem. Someone with free time should 
convert that to a struct mutex, say configfs_subsystem.su_mtx -- which is 
the preferred way to use (binary) mutexes presently. CC'ing Joel Becker on 
this.

---

Fix two races in fs/dlm/config.c:

(1) Grab the configfs subsystem semaphore before calling 
config_group_find_obj() in get_space(). This solves a potential race 
between get_space() and concurrent mkdir(2) or rmdir(2).

(2) Grab a reference on the found config_item _while_ holding the configfs 
subsystem semaphore in get_comm(), and not after it. This solves a 
potential race between get_comm() and concurrent rmdir(2).

  fs/dlm/config.c |   15 +++++++++++----
  1 file changed, 11 insertions(+), 4 deletions(-)

 	Signed-off-by: Satyam Sharma <ssatyam@....iitk.ac.in>

---

diff -ruNp linux-2.6.21.1/fs/dlm/config.c linux-2.6.21.1~patch/fs/dlm/config.c
--- linux-2.6.21.1/fs/dlm/config.c	2007-04-26 08:38:32.000000000 +0530
+++ linux-2.6.21.1~patch/fs/dlm/config.c	2007-05-04 21:08:54.000000000 +0530
@@ -744,9 +744,16 @@ static ssize_t node_weight_write(struct

  static struct space *get_space(char *name)
  {
+	struct config_item *i;
+
  	if (!space_list)
  		return NULL;
-	return to_space(config_group_find_obj(space_list, name));
+
+	down(&space_list->cg_subsys->su_sem);
+	i = config_group_find_obj(space_list, name);
+	up(&space_list->cg_subsys->su_sem);
+
+	return to_space(i);
  }

  static void put_space(struct space *sp)
@@ -772,20 +779,20 @@ static struct comm *get_comm(int nodeid,
  			if (cm->nodeid != nodeid)
  				continue;
  			found = 1;
+			config_item_get(i);
  			break;
  		} else {
  			if (!cm->addr_count ||
  			    memcmp(cm->addr[0], addr, sizeof(*addr)))
  				continue;
  			found = 1;
+			config_item_get(i);
  			break;
  		}
  	}
  	up(&clusters_root.subsys.su_sem);

-	if (found)
-		config_item_get(i);
-	else
+	if (!found)
  		cm = NULL;
  	return cm;
  }

-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/