Pull orangefs fixes from Mike Marshall: "Orangefs cleanups and a strncpy vulnerability fix. Cleanups: - remove an unused variable from orangefs_readdir. - clean up printk wrapper used for ofs "gossip" debugging. - clean up truncate ctime and mtime setting in inode.c - remove a useless null check found by coccinelle. - optimize some memcpy/memset boilerplate code. - remove some useless sanity checks from xattr.c Fix: - fix a potential strncpy vulnerability" * tag 'for-linus-4.6-ofs1' of git://git.kernel.org/pub/scm/linux/kernel/git/hubcap/linux: orangefs: remove unused variable orangefs: Add KERN_<LEVEL> to gossip_<level> macros orangefs: strncpy -> strscpy orangefs: clean up truncate ctime and mtime setting Orangefs: fix ifnullfree.cocci warnings Orangefs: optimize boilerplate code. Orangefs: xattr.c cleanup
Unified · Split
| @@ -153,7 +153,6 @@ static int orangefs_readdir(struct file *file, struct dir_context *ctx) · 152 unmodified lines | ||
| 153 | 153 | struct dentry *dentry = file->f_path.dentry; |
| 154 | 154 | struct orangefs_kernel_op_s *new_op = NULL; |
| 155 | 155 | struct orangefs_inode_s *orangefs_inode = ORANGEFS_I(dentry->d_inode); |
| 156 | − int buffer_full = 0; | |
| 157 | 156 | struct orangefs_readdir_response_s readdir_response; |
| 158 | 157 | void *dents_buf; |
| 159 | 158 | int i = 0; |
| @@ -350,8 +349,7 @@ get_new_buffer_index: · 190 unmodified lines | ||
| 350 | 349 | /* |
| 351 | 350 | * Did we hit the end of the directory? |
| 352 | 351 | */ |
| 353 | − if (readdir_response.token == ORANGEFS_READDIR_END && | |
| 354 | − !buffer_full) { | |
| 352 | + if (readdir_response.token == ORANGEFS_READDIR_END) { | |
| 355 | 353 | gossip_debug(GOSSIP_DIR_DEBUG, |
| 356 | 354 | "End of dir detected; setting ctx->pos to ORANGEFS_READDIR_END.\n"); |
| 357 | 355 | ctx->pos = ORANGEFS_READDIR_END; |
| @@ -204,22 +204,8 @@ static int orangefs_setattr_size(struct inode *inode, struct iattr *iattr) · 203 unmodified lines | ||
| 204 | 204 | if (ret != 0) |
| 205 | 205 | return ret; |
| 206 | 206 | |
| 207 | − /* | |
| 208 | − * Only change the c/mtime if we are changing the size or we are | |
| 209 | − * explicitly asked to change it. This handles the semantic difference | |
| 210 | − * between truncate() and ftruncate() as implemented in the VFS. | |
| 211 | − * | |
| 212 | − * The regular truncate() case without ATTR_CTIME and ATTR_MTIME is a | |
| 213 | − * special case where we need to update the times despite not having | |
| 214 | − * these flags set. For all other operations the VFS set these flags | |
| 215 | − * explicitly if it wants a timestamp update. | |
| 216 | − */ | |
| 217 | − if (orig_size != i_size_read(inode) && | |
| 218 | − !(iattr->ia_valid & (ATTR_CTIME | ATTR_MTIME))) { | |
| 219 | − iattr->ia_ctime = iattr->ia_mtime = | |
| 220 | − current_fs_time(inode->i_sb); | |
| 207 | + if (orig_size != i_size_read(inode)) | |
| 221 | 208 | iattr->ia_valid |= ATTR_CTIME | ATTR_MTIME; |
| 222 | − } | |
| 223 | 209 | |
| 224 | 210 | return ret; |
| 225 | 211 | } |
| @@ -126,8 +126,7 @@ out: · 125 unmodified lines | ||
| 126 | 126 | |
| 127 | 127 | void orangefs_debugfs_cleanup(void) |
| 128 | 128 | { |
| 129 | − if (debug_dir) | |
| 130 | − debugfs_remove_recursive(debug_dir); | |
| 129 | + debugfs_remove_recursive(debug_dir); | |
| 131 | 130 | } |
| 132 | 131 | |
| 133 | 132 | /* open ORANGEFS_KMOD_DEBUG_HELP_FILE */ |
| @@ -315,9 +315,13 @@ int orangefs_inode_getattr(struct inode *inode, int new, int size) · 314 unmodified lines | ||
| 315 | 315 | inode->i_size = (loff_t)strlen(new_op-> |
| 316 | 316 | downcall.resp.getattr.link_target); |
| 317 | 317 | orangefs_inode->blksize = (1 << inode->i_blkbits); |
| 318 | − strlcpy(orangefs_inode->link_target, | |
| 318 | + ret = strscpy(orangefs_inode->link_target, | |
| 319 | 319 | new_op->downcall.resp.getattr.link_target, |
| 320 | 320 | ORANGEFS_NAME_MAX); |
| 321 | + if (ret == -E2BIG) { | |
| 322 | + ret = -EIO; | |
| 323 | + goto out; | |
| 324 | + } | |
| 321 | 325 | inode->i_link = orangefs_inode->link_target; |
| 322 | 326 | } |
| 323 | 327 | break; |
| @@ -1,3 +1,4 @@ | ||
| 1 | +#include <linux/kernel.h> | |
| 1 | 2 | #include <linux/types.h> |
| 2 | 3 | #include <linux/spinlock_types.h> |
| 3 | 4 | #include <linux/slab.h> |
| @@ -74,8 +75,8 @@ static inline void ORANGEFS_khandle_to(const struct orangefs_khandle *kh, · 70 unmodified lines | ||
| 74 | 75 | void *p, int size) |
| 75 | 76 | { |
| 76 | 77 | |
| 77 | − memset(p, 0, size); | |
| 78 | 78 | memcpy(p, kh->u, 16); |
| 79 | + memset(p + 16, 0, size - 16); | |
| 79 | 80 | |
| 80 | 81 | } |
| 81 | 82 | |
| @@ -427,26 +428,28 @@ struct ORANGEFS_dev_map_desc { · 345 unmodified lines | ||
| 427 | 428 | /* gossip.h *****************************************************************/ |
| 428 | 429 | |
| 429 | 430 | #ifdef GOSSIP_DISABLE_DEBUG |
| 430 | −#define gossip_debug(mask, format, f...) do {} while (0) | |
| 431 | +#define gossip_debug(mask, fmt, ...) \ | |
| 432 | +do { \ | |
| 433 | + if (0) \ | |
| 434 | + printk(KERN_DEBUG fmt, ##__VA_ARGS__); \ | |
| 435 | +} while (0) | |
| 431 | 436 | #else |
| 432 | 437 | extern __u64 gossip_debug_mask; |
| 433 | 438 | extern struct client_debug_mask client_debug_mask; |
| 434 | 439 | |
| 435 | 440 | /* try to avoid function call overhead by checking masks in macro */ |
| 436 | −#define gossip_debug(mask, format, f...) \ | |
| 437 | −do { \ | |
| 438 | − if (gossip_debug_mask & mask) \ | |
| 439 | − printk(format, ##f); \ | |
| 441 | +#define gossip_debug(mask, fmt, ...) \ | |
| 442 | +do { \ | |
| 443 | + if (gossip_debug_mask & (mask)) \ | |
| 444 | + printk(KERN_DEBUG fmt, ##__VA_ARGS__); \ | |
| 440 | 445 | } while (0) |
| 441 | 446 | #endif /* GOSSIP_DISABLE_DEBUG */ |
| 442 | 447 | |
| 443 | 448 | /* do file and line number printouts w/ the GNU preprocessor */ |
| 444 | −#define gossip_ldebug(mask, format, f...) \ | |
| 445 | − gossip_debug(mask, "%s: " format, __func__, ##f) | |
| 446 | − | |
| 447 | −#define gossip_err printk | |
| 448 | −#define gossip_lerr(format, f...) \ | |
| 449 | − gossip_err("%s line %d: " format, \ | |
| 450 | − __FILE__, \ | |
| 451 | − __LINE__, \ | |
| 452 | − ##f) | |
| 449 | +#define gossip_ldebug(mask, fmt, ...) \ | |
| 450 | + gossip_debug(mask, "%s: " fmt, __func__, ##__VA_ARGS__) | |
| 451 | + | |
| 452 | +#define gossip_err pr_err | |
| 453 | +#define gossip_lerr(fmt, ...) \ | |
| 454 | + gossip_err("%s line %d: " fmt, \ | |
| 455 | + __FILE__, __LINE__, ##__VA_ARGS__) | |
| @@ -73,10 +73,6 @@ ssize_t orangefs_inode_getxattr(struct inode *inode, const char *prefix, · 72 unmodified lines | ||
| 73 | 73 | "%s: prefix %s name %s, buffer_size %zd\n", |
| 74 | 74 | __func__, prefix, name, size); |
| 75 | 75 | |
| 76 | − if (name == NULL || (size > 0 && buffer == NULL)) { | |
| 77 | − gossip_err("orangefs_inode_getxattr: bogus NULL pointers\n"); | |
| 78 | − return -EINVAL; | |
| 79 | − } | |
| 80 | 76 | if ((strlen(name) + strlen(prefix)) >= ORANGEFS_MAX_XATTR_NAMELEN) { |
| 81 | 77 | gossip_err("Invalid key length (%d)\n", |
| 82 | 78 | (int)(strlen(name) + strlen(prefix))); |
| @@ -146,8 +142,8 @@ ssize_t orangefs_inode_getxattr(struct inode *inode, const char *prefix, · 63 unmodified lines | ||
| 146 | 142 | goto out_release_op; |
| 147 | 143 | } |
| 148 | 144 | |
| 149 | − memset(buffer, 0, size); | |
| 150 | 145 | memcpy(buffer, new_op->downcall.resp.getxattr.val, length); |
| 146 | + memset(buffer + length, 0, size - length); | |
| 151 | 147 | gossip_debug(GOSSIP_XATTR_DEBUG, |
| 152 | 148 | "orangefs_inode_getxattr: inode %pU " |
| 153 | 149 | "key %s key_sz %d, val_len %d\n", |
| @@ -239,8 +235,7 @@ int orangefs_inode_setxattr(struct inode *inode, const char *prefix, · 85 unmodified lines | ||
| 239 | 235 | "%s: prefix %s, name %s, buffer_size %zd\n", |
| 240 | 236 | __func__, prefix, name, size); |
| 241 | 237 | |
| 242 | − if (size < 0 || | |
| 243 | − size >= ORANGEFS_MAX_XATTR_VALUELEN || | |
| 238 | + if (size >= ORANGEFS_MAX_XATTR_VALUELEN || | |
| 244 | 239 | flags < 0) { |
| 245 | 240 | gossip_err("orangefs_inode_setxattr: bogus values of size(%d), flags(%d)\n", |
| 246 | 241 | (int)size, |
| @@ -248,12 +243,6 @@ int orangefs_inode_setxattr(struct inode *inode, const char *prefix, · 1 unmodified line | ||
| 248 | 243 | return -EINVAL; |
| 249 | 244 | } |
| 250 | 245 | |
| 251 | − if (name == NULL || | |
| 252 | − (size > 0 && value == NULL)) { | |
| 253 | − gossip_err("orangefs_inode_setxattr: bogus NULL pointers!\n"); | |
| 254 | − return -EINVAL; | |
| 255 | − } | |
| 256 | − | |
| 257 | 246 | internal_flag = convert_to_internal_xattr_flags(flags); |
| 258 | 247 | |
| 259 | 248 | if (prefix) { |
| @@ -353,10 +342,6 @@ ssize_t orangefs_listxattr(struct dentry *dentry, char *buffer, size_t size) · 93 unmodified lines | ||
| 353 | 342 | gossip_err("%s: bogus NULL pointers\n", __func__); |
| 354 | 343 | return -EINVAL; |
| 355 | 344 | } |
| 356 | − if (size < 0) { | |
| 357 | − gossip_err("Invalid size (%d)\n", (int)size); | |
| 358 | − return -EINVAL; | |
| 359 | − } | |
| 360 | 345 | |
| 361 | 346 | down_read(&orangefs_inode->xattr_sem); |
| 362 | 347 | new_op = op_alloc(ORANGEFS_VFS_OP_LISTXATTR); |