Skip to contentSkip to conversation

fs: NULL dereference in posix_acl_to_xattr()

PATCH In review Dan Carpenter 2 participants 3 messages Feb 14, 2014 – Feb 25, 2014

Subsystems (2)
VFS Kernel janitors
lore
Share & tools
lore

Dan Carpenter

author
#0 lore raw

PATCH 1/1 fs: NULL dereference in posix_acl_to_xattr()

This patch moves the dereference of "buffer" after the check for NULL. The only place which passes a NULL parameter is gfs2_set_acl().

Cc stable <[email protected]>
Signed-off-by Dan Carpenter <[email protected]>
fs/posix_acl.c +3 -2
index 11c54fd51e16..9e363e41dacc 100644
@@ -723,7 +723,7 @@ posix_acl_to_xattr(struct user_namespace *user_ns, const struct posix_acl *acl,
 		   void *buffer, size_t size)
 {
 	posix_acl_xattr_header *ext_acl = (posix_acl_xattr_header *)buffer;
-	posix_acl_xattr_entry *ext_entry = ext_acl->a_entries;
+	posix_acl_xattr_entry *ext_entry;
 	int real_size, n;
 
 	real_size = posix_acl_xattr_size(acl->a_count);

@@ -731,7 +731,8 @@ posix_acl_to_xattr(struct user_namespace *user_ns, const struct posix_acl *acl,
 		return real_size;
 	if (real_size > size)
 		return -ERANGE;
-	
+
+	ext_entry = ext_acl->a_entries;
 	ext_acl->a_version = cpu_to_le32(POSIX_ACL_XATTR_VERSION);
 
 	for (n=0; n < acl->a_count; n++, ext_entry++) {
#1 lore raw

Hi,

Quoted 6 lines · Dan Carpenter

This patch moves the dereference of "buffer" after the check for NULL. The only place which passes a NULL parameter is gfs2_set_acl().

Cc stable <[email protected]>
Signed-off-by Dan Carpenter <[email protected]>

Al, are you going to take this, or should I put it through the GFS2 tree instead?

I also wonder whether GFS2 should be calling posix_acl_xattr_size() like the other filesystems, or whether in fact calling posix_acl_to_xattr() with a NULL buffer is still accepted usage here?

Steve.

Quoted 23 lines
fs/posix_acl.c +3 -2
index 11c54fd51e16..9e363e41dacc 100644
@@ -723,7 +723,7 @@ posix_acl_to_xattr(struct user_namespace *user_ns, const struct posix_acl *acl,
 		   void *buffer, size_t size)
 {
 	posix_acl_xattr_header *ext_acl = (posix_acl_xattr_header *)buffer;
-	posix_acl_xattr_entry *ext_entry = ext_acl->a_entries;
+	posix_acl_xattr_entry *ext_entry;
 	int real_size, n;
 
 	real_size = posix_acl_xattr_size(acl->a_count);

@@ -731,7 +731,8 @@ posix_acl_to_xattr(struct user_namespace *user_ns, const struct posix_acl *acl,
 		return real_size;
 	if (real_size > size)
 		return -ERANGE;
-	
+
+	ext_entry = ext_acl->a_entries;
 	ext_acl->a_version = cpu_to_le32(POSIX_ACL_XATTR_VERSION);
 
 	for (n=0; n < acl->a_count; n++, ext_entry++) {
#2 lore raw
In reply to Dan Carpenter

Hi,

Quoted 6 lines · Dan Carpenter

This patch moves the dereference of "buffer" after the check for NULL. The only place which passes a NULL parameter is gfs2_set_acl().

Cc stable <[email protected]>
Signed-off-by Dan Carpenter <[email protected]>

I've not heard anything from Al, so at least for the time being, I'm putting this in the GFS2 tree, that way it will at least get some exposure in -next,

Steve.

Quoted 23 lines
fs/posix_acl.c +3 -2
index 11c54fd51e16..9e363e41dacc 100644
@@ -723,7 +723,7 @@ posix_acl_to_xattr(struct user_namespace *user_ns, const struct posix_acl *acl,
 		   void *buffer, size_t size)
 {
 	posix_acl_xattr_header *ext_acl = (posix_acl_xattr_header *)buffer;
-	posix_acl_xattr_entry *ext_entry = ext_acl->a_entries;
+	posix_acl_xattr_entry *ext_entry;
 	int real_size, n;
 
 	real_size = posix_acl_xattr_size(acl->a_count);

@@ -731,7 +731,8 @@ posix_acl_to_xattr(struct user_namespace *user_ns, const struct posix_acl *acl,
 		return real_size;
 	if (real_size > size)
 		return -ERANGE;
-	
+
+	ext_entry = ext_acl->a_entries;
 	ext_acl->a_version = cpu_to_le32(POSIX_ACL_XATTR_VERSION);
 
 	for (n=0; n < acl->a_count; n++, ext_entry++) {