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>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-Id: <11902770983349-git-send-email-htejun@gmail.com>
Date:	Thu, 20 Sep 2007 17:31:38 +0900
From:	Tejun Heo <htejun@...il.com>
To:	ebiederm@...ssion.com, cornelia.huck@...ibm.com, greg@...ah.com,
	stern@...land.harvard.edu, kay.sievers@...y.org,
	linux-kernel@...r.kernel.org, htejun@...il.com
Cc:	Tejun Heo <htejun@...il.com>
Subject: [PATCH 7/8] sysfs: implement batch error handling

Batch error handling is to be used with plugged creation.  It
accumulates any error condition which happens under a plugged node and
let the user handle error once right before unplugging.  This easily
replaces attribute group creation.

Batch error handling works by the following two mechanisms.

* When an error occurs, tree is walked toward the top and, if there
  are plugged nodes which don't have plugged error recorded yet,
  sd->s_batch_error is set before returning the error code.

* All interface funtions allow ERR_PTR value to be passed as
  sysfs_dirent arguments and return -EBADF if that happens.

* After all the operations are complete, the user calls
  sysfs_check_batch_error() on the plugged node (usually the top one)
  which returns 0 if there hasn't been any error or error code of the
  first error.

* The user unplugs or removes the node accordingly.

Signed-off-by: Tejun Heo <htejun@...il.com>
---
 fs/sysfs/bin.c        |    8 +++-
 fs/sysfs/dir.c        |  137 ++++++++++++++++++++++++++++++++++++++++++++++--
 fs/sysfs/file.c       |   12 ++++-
 fs/sysfs/symlink.c    |    7 +++
 fs/sysfs/sysfs.h      |   12 ++++-
 include/linux/sysfs.h |    6 ++
 6 files changed, 173 insertions(+), 9 deletions(-)

diff --git a/fs/sysfs/bin.c b/fs/sysfs/bin.c
index d6cc7b3..1f705d2 100644
--- a/fs/sysfs/bin.c
+++ b/fs/sysfs/bin.c
@@ -231,10 +231,16 @@ struct sysfs_dirent *sysfs_add_bin(struct sysfs_dirent *parent,
 {
 	struct sysfs_dirent *sd;
 
+	/* for batch error handling */
+	if (IS_ERR(parent))
+		return ERR_PTR(-EBADF);
+
 	/* allocate and initialize */
 	sd = sysfs_new_dirent(name, mode, SYSFS_BIN);
-	if (!sd)
+	if (!sd) {
+		sysfs_set_batch_error(parent, -ENOMEM);
 		return ERR_PTR(-ENOMEM);
+	}
 
 	sd->s_bin.ops = bops;
 	sd->s_bin.size = size;
diff --git a/fs/sysfs/dir.c b/fs/sysfs/dir.c
index 88ec749..f18da1b 100644
--- a/fs/sysfs/dir.c
+++ b/fs/sysfs/dir.c
@@ -305,7 +305,12 @@ void release_sysfs_dirent(struct sysfs_dirent * sd)
 	if (sd->s_flags & SYSFS_FLAG_LINK_NAME_FMT_COPIED)
 		kfree(sd->s_link.name_fmt);
 
-	kfree(sd->s_iattr);
+	/* s_batch_error shares room with s_iattr.  If plugged, it's
+	 * s_batch_error; otherwise, s_iattr.
+	 */
+	if (!(sd->s_flags & SYSFS_FLAG_PLUGGED))
+		kfree(sd->s_iattr);
+
 	sysfs_free_ino(sd->s_ino);
 	kmem_cache_free(sysfs_dir_cachep, sd);
 
@@ -722,6 +727,7 @@ struct sysfs_dirent *sysfs_insert_one(struct sysfs_dirent *parent,
 
 	if (rc) {
 		sysfs_put(sd);
+		sysfs_set_batch_error(parent, rc);
 		return ERR_PTR(rc);
 	}
 
@@ -804,6 +810,10 @@ struct sysfs_dirent *sysfs_find_child(struct sysfs_dirent *parent,
 {
 	struct sysfs_dirent *sd;
 
+	/* for batch error handling */
+	if (IS_ERR(parent))
+		return ERR_PTR(-EBADF);
+
 	mutex_lock(&sysfs_mutex);
 	sd = sysfs_find_dirent(parent, name);
 	mutex_unlock(&sysfs_mutex);
@@ -837,10 +847,16 @@ struct sysfs_dirent *sysfs_add_dir(struct sysfs_dirent *parent,
 {
 	struct sysfs_dirent *sd;
 
+	/* for plugged error handling */
+	if (IS_ERR(parent))
+		return ERR_PTR(-EBADF);
+
 	/* allocate and initialize */
 	sd = sysfs_new_dirent(name, mode, SYSFS_DIR);
-	if (!sd)
+	if (!sd) {
+		sysfs_set_batch_error(parent, -ENOMEM);
 		return ERR_PTR(-ENOMEM);
+	}
 
 	sd->s_dir.data = data;
 
@@ -849,6 +865,71 @@ struct sysfs_dirent *sysfs_add_dir(struct sysfs_dirent *parent,
 EXPORT_SYMBOL_GPL(sysfs_add_dir);
 
 /**
+ *	sysfs_set_batch_error - record batch error
+ *	@sd: sysfs_dirent which failed operation
+ *	@error: error value
+ *
+ *	If @error is an error value, find all plugging ancestors and
+ *	record batch error status if this is the first error in the
+ *	subtree.  Note that only the first error behind a plugged node
+ *	is recorded.
+ *
+ *	LOCKING:
+ *	Kernel thread context (may sleep).  Uses sysfs_mutex.
+ */
+void sysfs_set_batch_error(struct sysfs_dirent *sd, int error)
+{
+	if (!IS_ERR_VALUE(error))
+		return;
+
+	mutex_lock(&sysfs_mutex);
+
+	for ( ; sd; sd = sd->s_parent) {
+		if (!(sd->s_flags & SYSFS_FLAG_PLUGGED) || sd->s_batch_error) {
+			BUG_ON(sd->s_batch_error &&
+			       !IS_ERR_VALUE(sd->s_batch_error));
+			continue;
+		}
+
+		/* s_iattr must be unused at this point */
+		BUG_ON(sd->s_iattr);
+		sd->s_batch_error = error;
+	}
+
+	mutex_unlock(&sysfs_mutex);
+}
+
+/**
+ *	sysfs_check_batch_error - check batch error
+ *	@sd: sysfs_dirent to check batch error for
+ *
+ *	Check whether any error has occurred on a plugged node @sd and
+ *	the nodes behind it.
+ *
+ *	LOCKING:
+ *	None.
+ *
+ *	RETURNS:
+ *	0 if there hasn't been any batch error, -errno if it had one
+ *	or more batch errors.  -errno is from the first batch error.
+ */
+int sysfs_check_batch_error(struct sysfs_dirent *sd)
+{
+	int error = 0;
+
+	if (IS_ERR(sd))
+		return PTR_ERR(sd);
+
+	if (sd->s_batch_error) {
+		error = sd->s_batch_error;
+		BUG_ON(!IS_ERR_VALUE(error));
+	}
+
+	return error;
+}
+EXPORT_SYMBOL_GPL(sysfs_check_batch_error);
+
+/**
  *	sysfs_unplug - unplug a plugged sysfs_dirent
  *	@sd: sysfs_dirent to unplug
  *
@@ -858,12 +939,19 @@ EXPORT_SYMBOL_GPL(sysfs_add_dir);
  *	userland.
  *
  *	LOCKING:
- *	Kernel thread context (may sleep).  Uses sysfs_mutex.
+ *	Kernel thread context (may sleep).  Uses sysfs_op_mutex and
+ *	sysfs_mutex.
  */
 void sysfs_unplug(struct sysfs_dirent *sd)
 {
 	struct sysfs_addrm_cxt acxt;
 
+	/* This happens when creation of the top node failed but the
+	 * user want to ignore it.  Do nothing.
+	 */
+	if (IS_ERR(sd))
+		return;
+
 	/* Unplugging performs parent inode update delayed from
 	 * add/removal.  Also, sysfs_op_mutex grabbed by addrm_start
 	 * guarantees renaming to see consistent plugged status.
@@ -876,6 +964,12 @@ void sysfs_unplug(struct sysfs_dirent *sd)
 
 		sd->s_flags &= ~SYSFS_FLAG_PLUGGED;
 
+		/* s_batch_error shares room with s_iattr, make sure
+		 * it's NULL.
+		 */
+		BUG_ON(sd->s_batch_error && !IS_ERR_VALUE(sd->s_batch_error));
+		sd->s_iattr = NULL;
+
 		if (parent_inode) {
 			parent_inode->i_ctime =
 			parent_inode->i_mtime = CURRENT_TIME;
@@ -1036,8 +1130,8 @@ void __sysfs_remove(struct sysfs_dirent *sd, int recurse)
 	struct sysfs_addrm_cxt acxt;
 	struct sysfs_dirent *cur, *next;
 
-	/* noop on NULL */
-	if (!sd)
+	/* noop on NULL and ERR_PTR, the latter is for batch error handling */
+	if (!sd || IS_ERR(sd))
 		return;
 
 	sysfs_addrm_start(&acxt);
@@ -1223,6 +1317,18 @@ static int sysfs_rcxt_get_dentries(struct sysfs_rename_context *rcxt,
 	struct dentry *old_dentry, *new_parent_dentry, *new_dentry;
 	int rc;
 
+	/* If sd is plugged currently and will stay plugged under the
+	 * new parent, dentries are guaranteed to not exist and stay
+	 * that way till rename is complete (op mutex released) and
+	 * there's nothing to do regarding dentries.
+	 *
+	 * Changing plugged status by moving isn't allowed and this
+	 * function will fail later if the user tries to do that.
+	 */
+	if (sysfs_plugged(rent->sd, NULL) &&
+	    sysfs_plugged(rent->sd, rent->new_parent))
+		return 0;
+
 	/* get old dentry */
 	old_dentry = sysfs_get_dentry(rent->sd);
 	if (IS_ERR(old_dentry))
@@ -1424,6 +1530,10 @@ int sysfs_rename(struct sysfs_dirent *sd, struct sysfs_dirent *new_parent,
 	struct sysfs_rcxt_rename_ent *rent;
 	int error;
 
+	/* for batch error handling */
+	if (IS_ERR(sd) || IS_ERR(new_parent))
+		return -EBADF;
+
 	mutex_lock(&sysfs_op_mutex);
 
 	if (!new_parent)
@@ -1463,6 +1573,12 @@ int sysfs_rename(struct sysfs_dirent *sd, struct sysfs_dirent *new_parent,
 			sysfs_link_sibling(rent->sd);
 		}
 
+		/* no dentry, no inode - this happens for plugged rename */
+		if (!rent->old_dentry) {
+			WARN_ON(rent->new_dentry);
+			continue;
+		}
+
 		/* update dcache and inode accordingly */
 		if (sysfs_type(rent->sd) == SYSFS_DIR) {
 			drop_nlink(rent->old_dentry->d_parent->d_inode);
@@ -1479,6 +1595,8 @@ int sysfs_rename(struct sysfs_dirent *sd, struct sysfs_dirent *new_parent,
 	sysfs_post_rename(&rcxt, error);
  out:
 	mutex_unlock(&sysfs_op_mutex);
+	sysfs_set_batch_error(sd, error);
+	sysfs_set_batch_error(new_parent, error);
 	return error;
 }
 EXPORT_SYMBOL_GPL(sysfs_rename);
@@ -1498,11 +1616,17 @@ EXPORT_SYMBOL_GPL(sysfs_rename);
  */
 int sysfs_chmod(struct sysfs_dirent *sd, mode_t mode)
 {
-	mode_t new_mode = (mode & S_IALLUGO) | (sd->s_mode & ~S_IALLUGO);
 	struct dentry *dentry = NULL;
 	struct inode *inode = NULL;
+	mode_t new_mode;
 	int rc = 0;
 
+	/* for batch error handling */
+	if (IS_ERR(sd))
+		return -EBADF;
+
+	new_mode = (mode & S_IALLUGO) | (sd->s_mode & ~S_IALLUGO);
+
 	mutex_lock(&sysfs_op_mutex);
 
 	/* If @sd is plugged, dentry and inode aren't there and can't
@@ -1537,6 +1661,7 @@ int sysfs_chmod(struct sysfs_dirent *sd, mode_t mode)
  out_unlock_op:
 	mutex_unlock(&sysfs_op_mutex);
 	dput(dentry);
+	sysfs_set_batch_error(sd, rc);
 	return rc;
 }
 EXPORT_SYMBOL_GPL(sysfs_chmod);
diff --git a/fs/sysfs/file.c b/fs/sysfs/file.c
index 1d93940..3412732 100644
--- a/fs/sysfs/file.c
+++ b/fs/sysfs/file.c
@@ -498,6 +498,10 @@ void sysfs_notify_file(struct sysfs_dirent *sd)
 {
 	struct sysfs_open_dirent *od;
 
+	/* for batch error handling */
+	if (IS_ERR(sd))
+		return;
+
 	BUG_ON(sysfs_type(sd) != SYSFS_FILE);
 
 	spin_lock(&sysfs_open_dirent_lock);
@@ -545,10 +549,16 @@ struct sysfs_dirent *sysfs_add_file(struct sysfs_dirent *parent,
 {
 	struct sysfs_dirent *sd;
 
+	/* for batch error handling */
+	if (IS_ERR(parent))
+		return ERR_PTR(-EBADF);
+
 	/* allocate and initialize */
 	sd = sysfs_new_dirent(name, mode, SYSFS_FILE);
-	if (!sd)
+	if (!sd) {
+		sysfs_set_batch_error(parent, -ENOMEM);
 		return ERR_PTR(-ENOMEM);
+	}
 
 	sd->s_file.ops = fops;
 	sd->s_file.data = data;
diff --git a/fs/sysfs/symlink.c b/fs/sysfs/symlink.c
index 53cc8a6..7e0fd89 100644
--- a/fs/sysfs/symlink.c
+++ b/fs/sysfs/symlink.c
@@ -217,6 +217,10 @@ struct sysfs_dirent *__sysfs_add_link(struct sysfs_dirent *parent,
 	struct sysfs_addrm_cxt acxt;
 	int rc;
 
+	/* for batch error handling */
+	if (IS_ERR(parent) || IS_ERR(target))
+		return ERR_PTR(-EBADF);
+
 	/* acquire locks early for link name formatting */
 	sysfs_addrm_start(&acxt);
 
@@ -280,6 +284,9 @@ struct sysfs_dirent *__sysfs_add_link(struct sysfs_dirent *parent,
 		kfree(name_fmt);
 	if (flags & SYSFS_FLAG_NAME_COPIED)
 		kfree(name);
+
+	sysfs_set_batch_error(parent, rc);
+	sysfs_set_batch_error(target, rc);
 	return ERR_PTR(rc);
 }
 
diff --git a/fs/sysfs/sysfs.h b/fs/sysfs/sysfs.h
index 51f346d..ec0b308 100644
--- a/fs/sysfs/sysfs.h
+++ b/fs/sysfs/sysfs.h
@@ -52,7 +52,15 @@ struct sysfs_dirent {
 	unsigned int		s_flags;
 	ino_t			s_ino;
 	umode_t			s_mode;
-	struct iattr		*s_iattr;
+
+	/* Make s_iattr and s_batch_error share the room.  This is
+	 * okay because s_iattr is used only after a node is unplugged
+	 * while s_batch_error is used only while a node is plugged.
+	 */
+	union {
+		struct iattr		*s_iattr;
+		int			s_batch_error;
+	};
 };
 
 #define SD_DEACTIVATED_BIAS		INT_MIN
@@ -120,6 +128,8 @@ struct sysfs_dirent *sysfs_get_dirent(struct sysfs_dirent *parent_sd,
 				      const unsigned char *name);
 struct sysfs_dirent *sysfs_new_dirent(const char *name, umode_t mode, int type);
 
+void sysfs_set_batch_error(struct sysfs_dirent *sd, int error);
+
 void __sysfs_remove(struct sysfs_dirent *sd, int recurse);
 
 void release_sysfs_dirent(struct sysfs_dirent *sd);
diff --git a/include/linux/sysfs.h b/include/linux/sysfs.h
index 5a8c9f1..a8dfc6b 100644
--- a/include/linux/sysfs.h
+++ b/include/linux/sysfs.h
@@ -64,6 +64,7 @@ struct sysfs_dirent *sysfs_add_bin(struct sysfs_dirent *parent,
 struct sysfs_dirent *sysfs_add_link(struct sysfs_dirent *parent,
 			const char *name_fmt, mode_t mode,
 			struct sysfs_dirent *target);
+int sysfs_check_batch_error(struct sysfs_dirent *sd);
 void sysfs_unplug(struct sysfs_dirent *sd);
 
 struct sysfs_dirent *sysfs_find_child(struct sysfs_dirent *parent,
@@ -108,6 +109,11 @@ static inline struct sysfs_dirent *sysfs_add_link(struct sysfs_dirent *parent,
 	return NULL;
 }
 
+static inline int sysfs_check_batch_error(struct sysfs_dirent *sd)
+{
+	return 0;
+}
+
 static inline void sysfs_unplug(struct sysfs_dirent *sd)
 {
 }
-- 
1.5.0.3


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

Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ