On 01/01 2014 19:19 PM, Jeff Liu wrote: > From: Jie Liu <[email protected]> > > There is a potential overflow if the specified EA value size is > greater than USHRT_MAX because the size of value is limitted by > the on-disk format (i.e, __le16), this issue could be reflected > via the tests below: > # touch /jfs/testfile > # setfattr -n user.comment -v `perl -e 'print "A"x65536'` /jfs/testfile > setfattr: /jfs/testfile: Invalid argument > > Syslog: jfs_xsetattr: xattr_size = 21, new_size = 65557 > > This patch add pre-checkups of EA value size against USHRT_MAX to > avoid this problem, and return -E2BIG which is consistent with the > VFS setxattr interface. Moreover, fix the debug code to print the > correct function name. > > With this fix: > setfattr: /jfs/testfile: Argument list too long > > Signed-off-by: Jie Liu <[email protected]> > --- > fs/jfs/xattr.c | 13 ++++++++++++- > 1 file changed, 12 insertions(+), 1 deletion(-) > > diff --git a/fs/jfs/xattr.c b/fs/jfs/xattr.c > index d3472f4..21755fb 100644 > --- a/fs/jfs/xattr.c > +++ b/fs/jfs/xattr.c > @@ -860,6 +860,17 @@ int __jfs_setxattr(tid_t tid, struct inode *inode, const > char *name, > /* Completely new ea list */ > xattr_size = sizeof (struct jfs_ea_list); > > + /* > + * The size of EA value is limitted by on-disk format up to > + * __le16, there would be an overflow if the size is equal > + * to XATTR_SIZE_MAX (65536). In order to avoid this issue, > + * we can pre-checkup the value size against USHRT_MAX, and > + * return -E2BIG in this case, which is consistent with the > + * VFS setxattr interface. > + */ > + if (value_len >= USHRT_MAX) > + return -E2BIG; > + > ea = (struct jfs_ea *) ((char *) ealist + xattr_size); > ea->flag = 0; > ea->namelen = namelen; > @@ -874,7 +885,7 @@ int __jfs_setxattr(tid_t tid, struct inode *inode, const > char *name, > /* DEBUG - If we did this right, these number match */ > if (xattr_size != new_size) { > printk(KERN_ERR > - "jfs_xsetattr: xattr_size = %d, new_size = %d\n", > + "__jfs_setattr: xattr_size = %d, new_size = %d\n", Sorry Dave! here is a typo -- s/__jfs_setattr/__jfs_setxattr/.
The revised version is shown as following: From: Jie Liu <[email protected]> Subject: jfs: fix xattr value size overflow in __jfs_setxattr There is a potential overflow if the specified EA value size is greater than USHRT_MAX because the size of value is limitted by the on-disk format (i.e, __le16), this issue could be reflected via the tests below: # touch /jfs/testfile # setfattr -n user.comment -v `perl -e 'print "A"x65536'` /jfs/testfile setfattr: /jfs/testfile: Invalid argument Syslog: ... jfs_xsetattr: xattr_size = 21, new_size = 65557 This patch add pre-checkups of EA value size against USHRT_MAX to avoid this problem, and return -E2BIG which is consistent with the VFS setxattr interface. Moreover, fix the debug code to print the correct function name. With this fix: setfattr: /jfs/testfile: Argument list too long Signed-off-by: Jie Liu <[email protected]> --- fs/jfs/xattr.c | 13 ++++++++++++- 1 file changed, 12 insertions(+), 1 deletion(-) diff --git a/fs/jfs/xattr.c b/fs/jfs/xattr.c index d3472f4..21755fb 100644 --- a/fs/jfs/xattr.c +++ b/fs/jfs/xattr.c @@ -860,6 +860,17 @@ int __jfs_setxattr(tid_t tid, struct inode *inode, const char *name, /* Completely new ea list */ xattr_size = sizeof (struct jfs_ea_list); + /* + * The size of EA value is limitted by on-disk format up to + * __le16, there would be an overflow if the size is equal + * to XATTR_SIZE_MAX (65536). In order to avoid this issue, + * we can pre-checkup the value size against USHRT_MAX, and + * return -E2BIG in this case, which is consistent with the + * VFS setxattr interface. + */ + if (value_len >= USHRT_MAX) + return -E2BIG; + ea = (struct jfs_ea *) ((char *) ealist + xattr_size); ea->flag = 0; ea->namelen = namelen; @@ -874,7 +885,7 @@ int __jfs_setxattr(tid_t tid, struct inode *inode, const char *name, /* DEBUG - If we did this right, these number match */ if (xattr_size != new_size) { printk(KERN_ERR - "jfs_xsetattr: xattr_size = %d, new_size = %d\n", + "__jfs_setxattr: xattr_size = %d, new_size = %d\n", xattr_size, new_size); rc = -EINVAL; -- 1.8.3.2 ------------------------------------------------------------------------------ Rapidly troubleshoot problems before they affect your business. Most IT organizations don't have a clear picture of how application performance affects their revenue. With AppDynamics, you get 100% visibility into your Java,.NET, & PHP application. Start your 15-day FREE TRIAL of AppDynamics Pro! http://pubads.g.doubleclick.net/gampad/clk?id=84349831&iu=/4140/ostg.clktrk _______________________________________________ Jfs-discussion mailing list [email protected] https://lists.sourceforge.net/lists/listinfo/jfs-discussion
