linux-kernel - [PATCH] race condition fixing in sysfs_create

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: Windows password security audit tool. GUI, reports in PDF.

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

Message-ID: <51F24612.6030509@tnsoft.com.cn>
Date:	Fri, 26 Jul 2013 17:49:06 +0800
From:	Dennis Chen <xschen@...oft.com.cn>
To:	linux-kernel@...r.kernel.org
CC:	gregkh@...uxfoundation.org, teheo@...e.de,
	xiyou.wangcong@...il.com, Dennis Chen <xschen@...oft.com.cn>
Subject: [PATCH] race condition fixing in sysfs_create_dir

The patch is trying its best to avoid creating a dir under a parent dir which is removing from
the system:

     PATH0 (create a dir under 'PARENT/...')         PATH1 (remove the 'PARENT/...')
          sysfs_create_dir() {                         sysfs_remove_dir() {
          ...                                          ...
          if (kobj->parent)                            spin_lock(&sysfs_assoc_lock);
		parent_sd = kobj->parent->sd;  <----- kobj->sd = NULL;
	 else                                         spin_unlock(&sysfs_assoc_lock);
		parent_sd = &sysfs_root;

Suppose PATH1 enter the critical section first, then PATH0 begin to execute before kobj->sd
has been reset to NULL, possibly PATH0 will get a non-NULL parent_sd since lack of the
sysfs_assoc_lock protection in PATH0. In this case, PATH0 think it has a valid parent_sd which
can be freed by PATH1 in the followed, refer to the comments in the patch. Maybe we need
to figure out a perfect solution to solve the race condition, although the codes in question are
in slow path...

Signed-off-by: Dennis Chen <xschen@...oft.com.cn>
Cc: Greg Kroah-Hartman <gregkh@...uxfoundation.org>
Cc: Tejun Heo <teheo@...e.de>
Cc: Wang Cong <xiyou.wangcong@...il.com>
---
fs/sysfs/dir.c |   11 ++++++++++-
1 file changed, 10 insertions(+), 1 deletion(-)

diff --git a/fs/sysfs/dir.c b/fs/sysfs/dir.c
index e068e74..114073d 100644
--- a/fs/sysfs/dir.c
+++ b/fs/sysfs/dir.c
@@ -746,13 +746,22 @@ int sysfs_create_dir(struct kobject * kobj)
         BUG_ON(!kobj);

+       spin_lock(&sysfs_assoc_lock);
	if (kobj->parent)
		parent_sd = kobj->parent->sd;
	else
		parent_sd = &sysfs_root;

-       if (!parent_sd)
+       if (!parent_sd) {
+               spin_unlock(&sysfs_assoc_lock);
                 return -ENOENT;
+       }
+       spin_unlock(&sysfs_assoc_lock);
+       /* TODO: although the sysfs is in a slow path, but in the operation
+       * followed, we still have a window to let the sysfs_remove_dir to
+       * free the memory space pointered by parent_sd till we inc its ref
+       * count in __sysfs_add_one()
+       */

         if (sysfs_ns_type(parent_sd))
                 ns = kobj->ktype->namespace(kobj);

-- 

1.7.10.4

--
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/