public inbox for linux-kernel@vger.kernel.org
 help / color / mirror / Atom feed
From: Andreas Gruenbacher <agruen@suse.de>
To: James Morris <jmorris@redhat.com>
Cc: Andrew Morton <akpm@osdl.org>,
	Al Viro <viro@parcelfarce.linux.theplanet.co.uk>,
	Stephen Smalley <sds@epoch.ncsc.mil>,
	Christoph Hellwig <hch@infradead.org>,
	"linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>
Subject: Re: [PATCH 0/6] xattr consolidation v3
Date: Sat, 25 Sep 2004 00:59:11 +0200	[thread overview]
Message-ID: <200409250059.12129.agruen@suse.de> (raw)
In-Reply-To: <Xine.LNX.4.44.0409241237560.8101-100000@thoron.boston.redhat.com>

[-- Attachment #1: Type: text/plain, Size: 637 bytes --]

On Friday 24 September 2004 18:39, James Morris wrote:
> On Thu, 23 Sep 2004, Andreas Gruenbacher wrote:
> > There is one minor issue in the ext[23]_xattr_list changes: The xattr
> > block gets inserted into the cache even if it later turns out to be
> > corrupted. The attached patch reintroduces the sanity check, and has a
> > few other cosmetical-only changes.
> >
> > Andrew, do you want to add this to -mm?
>
> These 'cosmetical-only' changes break listxattr on ext3 and ext2.  Andrew,
> please drop this patch.

Here is a fixed version. Feedback welcome.

Cheers,
-- 
Andreas Gruenbacher <agruen@suse.de>
SUSE Labs, SUSE LINUX AG

[-- Attachment #2: delta-2.diff --]
[-- Type: text/x-diff, Size: 8875 bytes --]

Index: linux-2.6.9-rc2-bk10/fs/ext2/acl.c
===================================================================
--- linux-2.6.9-rc2-bk10.orig/fs/ext2/acl.c
+++ linux-2.6.9-rc2-bk10/fs/ext2/acl.c
@@ -436,7 +436,7 @@ ext2_xattr_list_acl_access(struct inode 
 
 	if (!test_opt(inode->i_sb, POSIX_ACL))
 		return 0;
-	if (list && (size <= list_size))
+	if (list && size <= list_size)
 		memcpy(list, XATTR_NAME_ACL_ACCESS, size);
 	return size;
 }
@@ -449,7 +449,7 @@ ext2_xattr_list_acl_default(struct inode
 
 	if (!test_opt(inode->i_sb, POSIX_ACL))
 		return 0;
-	if (list && (size <= list_size))
+	if (list && size <= list_size)
 		memcpy(list, XATTR_NAME_ACL_DEFAULT, size);
 	return size;
 }
Index: linux-2.6.9-rc2-bk10/fs/ext2/xattr.c
===================================================================
--- linux-2.6.9-rc2-bk10.orig/fs/ext2/xattr.c
+++ linux-2.6.9-rc2-bk10/fs/ext2/xattr.c
@@ -270,8 +270,8 @@ ext2_xattr_list(struct inode *inode, cha
 {
 	struct buffer_head *bh = NULL;
 	struct ext2_xattr_entry *entry;
-	size_t total_size = 0;
-	char *buf, *end;
+	char *end;
+	size_t rest = buffer_size;
 	int error;
 
 	ea_idebug(inode, "buffer=%p, buffer_size=%ld",
@@ -298,36 +298,39 @@ bad_block:	ext2_error(inode->i_sb, "ext2
 		goto cleanup;
 	}
 
+	/* check the on-disk data structure */
+	entry = FIRST_ENTRY(bh);
+	while (!IS_LAST_ENTRY(entry)) {
+		struct ext2_xattr_entry *next = EXT2_XATTR_NEXT(entry);
+
+		if ((char *)next >= end)
+			goto bad_block;
+		entry = next;
+	}
 	if (ext2_xattr_cache_insert(bh))
 		ea_idebug(inode, "cache insert failed");
 
 	/* list the attribute names */
-	buf = buffer;
 	for (entry = FIRST_ENTRY(bh); !IS_LAST_ENTRY(entry);
 	     entry = EXT2_XATTR_NEXT(entry)) {
-		struct xattr_handler *handler;
-		struct ext2_xattr_entry *next = EXT2_XATTR_NEXT(entry);
-		
-		if ((char *)next >= end)
-			goto bad_block;
-			
-		handler = ext2_xattr_handler(entry->e_name_index);
+		struct xattr_handler *handler =
+			ext2_xattr_handler(entry->e_name_index);
+
 		if (handler) {
-			size_t size = handler->list(inode, buf, buffer_size,
+			size_t size = handler->list(inode, buffer, rest,
 						    entry->e_name,
 						    entry->e_name_len);
-			if (buf) {
-				if (size > buffer_size) {
+			if (buffer) {
+				if (size > rest) {
 					error = -ERANGE;
 					goto cleanup;
 				}
-				buf += size;
-				buffer_size -= size;
+				buffer += size;
 			}
-			total_size += size;
+			rest -= size;
 		}
 	}
-	error = total_size;
+	error = buffer_size - rest;  /* total size */
 
 cleanup:
 	brelse(bh);
Index: linux-2.6.9-rc2-bk10/fs/ext2/xattr_security.c
===================================================================
--- linux-2.6.9-rc2-bk10.orig/fs/ext2/xattr_security.c
+++ linux-2.6.9-rc2-bk10/fs/ext2/xattr_security.c
@@ -17,7 +17,7 @@ ext2_xattr_security_list(struct inode *i
 	const int prefix_len = sizeof(XATTR_SECURITY_PREFIX)-1;
 	const size_t total_len = prefix_len + name_len + 1;
 	
-	if (list && (total_len <= list_size)) {
+	if (list && total_len <= list_size) {
 		memcpy(list, XATTR_SECURITY_PREFIX, prefix_len);
 		memcpy(list+prefix_len, name, name_len);
 		list[prefix_len + name_len] = '\0';
Index: linux-2.6.9-rc2-bk10/fs/ext2/xattr_trusted.c
===================================================================
--- linux-2.6.9-rc2-bk10.orig/fs/ext2/xattr_trusted.c
+++ linux-2.6.9-rc2-bk10/fs/ext2/xattr_trusted.c
@@ -24,7 +24,7 @@ ext2_xattr_trusted_list(struct inode *in
 	if (!capable(CAP_SYS_ADMIN))
 		return 0;
 
-	if (list && (total_len <= list_size)) {
+	if (list && total_len <= list_size) {
 		memcpy(list, XATTR_TRUSTED_PREFIX, prefix_len);
 		memcpy(list+prefix_len, name, name_len);
 		list[prefix_len + name_len] = '\0';
Index: linux-2.6.9-rc2-bk10/fs/ext2/xattr_user.c
===================================================================
--- linux-2.6.9-rc2-bk10.orig/fs/ext2/xattr_user.c
+++ linux-2.6.9-rc2-bk10/fs/ext2/xattr_user.c
@@ -23,7 +23,7 @@ ext2_xattr_user_list(struct inode *inode
 	if (!test_opt(inode->i_sb, XATTR_USER))
 		return 0;
 
-	if (list && (total_len <= list_size)) {
+	if (list && total_len <= list_size) {
 		memcpy(list, XATTR_USER_PREFIX, prefix_len);
 		memcpy(list+prefix_len, name, name_len);
 		list[prefix_len + name_len] = '\0';
Index: linux-2.6.9-rc2-bk10/fs/ext3/acl.c
===================================================================
--- linux-2.6.9-rc2-bk10.orig/fs/ext3/acl.c
+++ linux-2.6.9-rc2-bk10/fs/ext3/acl.c
@@ -459,7 +459,7 @@ ext3_xattr_list_acl_access(struct inode 
 
 	if (!test_opt(inode->i_sb, POSIX_ACL))
 		return 0;
-	if (list && (size <= list_len))
+	if (list && size <= list_len)
 		memcpy(list, XATTR_NAME_ACL_ACCESS, size);
 	return size;
 }
@@ -472,7 +472,7 @@ ext3_xattr_list_acl_default(struct inode
 
 	if (!test_opt(inode->i_sb, POSIX_ACL))
 		return 0;
-	if (list && (size <= list_len))
+	if (list && size <= list_len)
 		memcpy(list, XATTR_NAME_ACL_DEFAULT, size);
 	return size;
 }
Index: linux-2.6.9-rc2-bk10/fs/ext3/xattr.c
===================================================================
--- linux-2.6.9-rc2-bk10.orig/fs/ext3/xattr.c
+++ linux-2.6.9-rc2-bk10/fs/ext3/xattr.c
@@ -267,8 +267,8 @@ ext3_xattr_list(struct inode *inode, cha
 {
 	struct buffer_head *bh = NULL;
 	struct ext3_xattr_entry *entry;
-	size_t total_size = 0;
-	char *buf, *end;
+	char *end;
+	size_t rest = buffer_size;
 	int error;
 
 	ea_idebug(inode, "buffer=%p, buffer_size=%ld",
@@ -295,36 +295,39 @@ bad_block:	ext3_error(inode->i_sb, "ext3
 		goto cleanup;
 	}
 
+	/* check the on-disk data structure */
+	entry = FIRST_ENTRY(bh);
+	while (!IS_LAST_ENTRY(entry)) {
+		struct ext3_xattr_entry *next = EXT3_XATTR_NEXT(entry);
+
+		if ((char *)next >= end)
+			goto bad_block;
+		entry = next;
+	}
 	if (ext3_xattr_cache_insert(bh))
 		ea_idebug(inode, "cache insert failed");
 
 	/* list the attribute names */
-	buf = buffer;
 	for (entry = FIRST_ENTRY(bh); !IS_LAST_ENTRY(entry);
 	     entry = EXT3_XATTR_NEXT(entry)) {
-		struct xattr_handler *handler;
-		struct ext3_xattr_entry *next = EXT3_XATTR_NEXT(entry);
-
-		if ((char *)next >= end)
-			goto bad_block;
+		struct xattr_handler *handler =
+			ext3_xattr_handler(entry->e_name_index);
 
-		handler = ext3_xattr_handler(entry->e_name_index);
 		if (handler) {
-			size_t size = handler->list(inode, buf, buffer_size,
+			size_t size = handler->list(inode, buffer, rest,
 						    entry->e_name,
 						    entry->e_name_len);
-			if (buf) {
-				if (size > buffer_size) {
+			if (buffer) {
+				if (size > rest) {
 					error = -ERANGE;
 					goto cleanup;
 				}
-				buf += size;
-				buffer_size -= size;
+				buffer += size;
 			}
-			total_size += size;
+			rest -= size;
 		}
 	}
-	error = total_size;
+	error = buffer_size - rest;  /* total size */
 
 cleanup:
 	brelse(bh);
Index: linux-2.6.9-rc2-bk10/fs/ext3/xattr_security.c
===================================================================
--- linux-2.6.9-rc2-bk10.orig/fs/ext3/xattr_security.c
+++ linux-2.6.9-rc2-bk10/fs/ext3/xattr_security.c
@@ -19,7 +19,7 @@ ext3_xattr_security_list(struct inode *i
 	const size_t total_len = prefix_len + name_len + 1;
 	
 
-	if (list && (total_len <= list_size)) {
+	if (list && total_len <= list_size) {
 		memcpy(list, XATTR_SECURITY_PREFIX, prefix_len);
 		memcpy(list+prefix_len, name, name_len);
 		list[prefix_len + name_len] = '\0';
Index: linux-2.6.9-rc2-bk10/fs/ext3/xattr_trusted.c
===================================================================
--- linux-2.6.9-rc2-bk10.orig/fs/ext3/xattr_trusted.c
+++ linux-2.6.9-rc2-bk10/fs/ext3/xattr_trusted.c
@@ -25,7 +25,7 @@ ext3_xattr_trusted_list(struct inode *in
 	if (!capable(CAP_SYS_ADMIN))
 		return 0;
 
-	if (list && (total_len <= list_size)) {
+	if (list && total_len <= list_size) {
 		memcpy(list, XATTR_TRUSTED_PREFIX, prefix_len);
 		memcpy(list+prefix_len, name, name_len);
 		list[prefix_len + name_len] = '\0';
Index: linux-2.6.9-rc2-bk10/fs/ext3/xattr_user.c
===================================================================
--- linux-2.6.9-rc2-bk10.orig/fs/ext3/xattr_user.c
+++ linux-2.6.9-rc2-bk10/fs/ext3/xattr_user.c
@@ -25,7 +25,7 @@ ext3_xattr_user_list(struct inode *inode
 	if (!test_opt(inode->i_sb, XATTR_USER))
 		return 0;
 
-	if (list && (total_len <= list_size)) {
+	if (list && total_len <= list_size) {
 		memcpy(list, XATTR_USER_PREFIX, prefix_len);
 		memcpy(list+prefix_len, name, name_len);
 		list[prefix_len + name_len] = '\0';
Index: linux-2.6.9-rc2-bk10/fs/xattr.c
===================================================================
--- linux-2.6.9-rc2-bk10.orig/fs/xattr.c
+++ linux-2.6.9-rc2-bk10/fs/xattr.c
@@ -351,7 +351,8 @@ sys_fremovexattr(int fd, char __user *na
 }
 
 
-static const char *strcmp_prefix(const char *a, const char *a_prefix)
+static const char *
+strcmp_prefix(const char *a, const char *a_prefix)
 {
 	while (*a_prefix && *a == *a_prefix) {
 		a++;

  reply	other threads:[~2004-09-24 23:02 UTC|newest]

Thread overview: 5+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2004-09-23  3:53 [PATCH 0/6] xattr consolidation v3 James Morris
2004-09-23 13:26 ` Andreas Gruenbacher
2004-09-24 16:39   ` James Morris
2004-09-24 22:59     ` Andreas Gruenbacher [this message]
2004-09-25  1:01       ` James Morris

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=200409250059.12129.agruen@suse.de \
    --to=agruen@suse.de \
    --cc=akpm@osdl.org \
    --cc=hch@infradead.org \
    --cc=jmorris@redhat.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=sds@epoch.ncsc.mil \
    --cc=viro@parcelfarce.linux.theplanet.co.uk \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox