diff options
author | Eric Sandeen <sandeen@sandeen.net> | 2016-04-05 17:57:18 -0400 |
---|---|---|
committer | Dave Chinner <david@fromorbit.com> | 2016-04-05 17:57:18 -0400 |
commit | 2a6fba6d2311151598abaa1e7c9abd5f8d024a43 (patch) | |
tree | 6ff4d9edda91b530deff6386dd796e6e91e6e121 | |
parent | f55532a0c0b8bb6148f4e07853b876ef73bc69ca (diff) |
xfs: only return -errno or success from attr ->put_listent
Today, the put_listent formatters return either 1 or 0; if
they return 1, some callers treat this as an error and return
it up the stack, despite "1" not being a valid (negative)
error code.
The intent seems to be that if the input buffer is full,
we set seen_enough or set count = -1, and return 1;
but some callers check the return before checking the
seen_enough or count fields of the context.
Fix this by only returning non-zero for actual errors
encountered, and rely on the caller to first check the
return value, then check the values in the context to
decide what to do.
Signed-off-by: Eric Sandeen <sandeen@redhat.com>
Reviewed-by: Christoph Hellwig <hch@lst.de>
Signed-off-by: Dave Chinner <david@fromorbit.com>
-rw-r--r-- | fs/xfs/xfs_attr.h | 1 | ||||
-rw-r--r-- | fs/xfs/xfs_attr_list.c | 8 | ||||
-rw-r--r-- | fs/xfs/xfs_xattr.c | 14 |
3 files changed, 14 insertions, 9 deletions
diff --git a/fs/xfs/xfs_attr.h b/fs/xfs/xfs_attr.h index dd4824589470..234331227c0c 100644 --- a/fs/xfs/xfs_attr.h +++ b/fs/xfs/xfs_attr.h | |||
@@ -112,6 +112,7 @@ typedef struct attrlist_cursor_kern { | |||
112 | *========================================================================*/ | 112 | *========================================================================*/ |
113 | 113 | ||
114 | 114 | ||
115 | /* Return 0 on success, or -errno; other state communicated via *context */ | ||
115 | typedef int (*put_listent_func_t)(struct xfs_attr_list_context *, int, | 116 | typedef int (*put_listent_func_t)(struct xfs_attr_list_context *, int, |
116 | unsigned char *, int, int, unsigned char *); | 117 | unsigned char *, int, int, unsigned char *); |
117 | 118 | ||
diff --git a/fs/xfs/xfs_attr_list.c b/fs/xfs/xfs_attr_list.c index 4fa14820e2e2..c8be331a3196 100644 --- a/fs/xfs/xfs_attr_list.c +++ b/fs/xfs/xfs_attr_list.c | |||
@@ -108,16 +108,14 @@ xfs_attr_shortform_list(xfs_attr_list_context_t *context) | |||
108 | (int)sfe->namelen, | 108 | (int)sfe->namelen, |
109 | (int)sfe->valuelen, | 109 | (int)sfe->valuelen, |
110 | &sfe->nameval[sfe->namelen]); | 110 | &sfe->nameval[sfe->namelen]); |
111 | 111 | if (error) | |
112 | return error; | ||
112 | /* | 113 | /* |
113 | * Either search callback finished early or | 114 | * Either search callback finished early or |
114 | * didn't fit it all in the buffer after all. | 115 | * didn't fit it all in the buffer after all. |
115 | */ | 116 | */ |
116 | if (context->seen_enough) | 117 | if (context->seen_enough) |
117 | break; | 118 | break; |
118 | |||
119 | if (error) | ||
120 | return error; | ||
121 | sfe = XFS_ATTR_SF_NEXTENTRY(sfe); | 119 | sfe = XFS_ATTR_SF_NEXTENTRY(sfe); |
122 | } | 120 | } |
123 | trace_xfs_attr_list_sf_all(context); | 121 | trace_xfs_attr_list_sf_all(context); |
@@ -581,7 +579,7 @@ xfs_attr_put_listent( | |||
581 | trace_xfs_attr_list_full(context); | 579 | trace_xfs_attr_list_full(context); |
582 | alist->al_more = 1; | 580 | alist->al_more = 1; |
583 | context->seen_enough = 1; | 581 | context->seen_enough = 1; |
584 | return 1; | 582 | return 0; |
585 | } | 583 | } |
586 | 584 | ||
587 | aep = (attrlist_ent_t *)&context->alist[context->firstu]; | 585 | aep = (attrlist_ent_t *)&context->alist[context->firstu]; |
diff --git a/fs/xfs/xfs_xattr.c b/fs/xfs/xfs_xattr.c index 110f1d7d86b0..f2201299311f 100644 --- a/fs/xfs/xfs_xattr.c +++ b/fs/xfs/xfs_xattr.c | |||
@@ -146,7 +146,7 @@ __xfs_xattr_put_listent( | |||
146 | arraytop = context->count + prefix_len + namelen + 1; | 146 | arraytop = context->count + prefix_len + namelen + 1; |
147 | if (arraytop > context->firstu) { | 147 | if (arraytop > context->firstu) { |
148 | context->count = -1; /* insufficient space */ | 148 | context->count = -1; /* insufficient space */ |
149 | return 1; | 149 | return 0; |
150 | } | 150 | } |
151 | offset = (char *)context->alist + context->count; | 151 | offset = (char *)context->alist + context->count; |
152 | strncpy(offset, prefix, prefix_len); | 152 | strncpy(offset, prefix, prefix_len); |
@@ -221,11 +221,15 @@ xfs_xattr_put_listent( | |||
221 | } | 221 | } |
222 | 222 | ||
223 | ssize_t | 223 | ssize_t |
224 | xfs_vn_listxattr(struct dentry *dentry, char *data, size_t size) | 224 | xfs_vn_listxattr( |
225 | struct dentry *dentry, | ||
226 | char *data, | ||
227 | size_t size) | ||
225 | { | 228 | { |
226 | struct xfs_attr_list_context context; | 229 | struct xfs_attr_list_context context; |
227 | struct attrlist_cursor_kern cursor = { 0 }; | 230 | struct attrlist_cursor_kern cursor = { 0 }; |
228 | struct inode *inode = d_inode(dentry); | 231 | struct inode *inode = d_inode(dentry); |
232 | int error; | ||
229 | 233 | ||
230 | /* | 234 | /* |
231 | * First read the regular on-disk attributes. | 235 | * First read the regular on-disk attributes. |
@@ -239,7 +243,9 @@ xfs_vn_listxattr(struct dentry *dentry, char *data, size_t size) | |||
239 | context.firstu = context.bufsize; | 243 | context.firstu = context.bufsize; |
240 | context.put_listent = xfs_xattr_put_listent; | 244 | context.put_listent = xfs_xattr_put_listent; |
241 | 245 | ||
242 | xfs_attr_list_int(&context); | 246 | error = xfs_attr_list_int(&context); |
247 | if (error) | ||
248 | return error; | ||
243 | if (context.count < 0) | 249 | if (context.count < 0) |
244 | return -ERANGE; | 250 | return -ERANGE; |
245 | 251 | ||