| Message ID | e204841964263393974c193010c74648faa5a761.1487614974.git.dsterba@suse.com (mailing list archive) |
|---|---|
| State | Accepted |
| Headers | show |
On Mon, Feb 20, 2017 at 07:25:06PM +0100, David Sterba wrote: > The space check in btrfs_insert_xattr_item is duplicated in it's caller > (do_setxattr) so we won't hit the BUG_ON. Continuing without any check > could be disasterous so turn it to a proper error handling. > > Signed-off-by: David Sterba <dsterba@suse.com> > --- > fs/btrfs/dir-item.c | 3 ++- > 1 file changed, 2 insertions(+), 1 deletion(-) > > diff --git a/fs/btrfs/dir-item.c b/fs/btrfs/dir-item.c > index 724504a2d7ac..640801082533 100644 > --- a/fs/btrfs/dir-item.c > +++ b/fs/btrfs/dir-item.c > @@ -80,7 +80,8 @@ int btrfs_insert_xattr_item(struct btrfs_trans_handle *trans, > struct extent_buffer *leaf; > u32 data_size; > > - BUG_ON(name_len + data_len > BTRFS_MAX_XATTR_SIZE(root->fs_info)); > + if (name_len + data_len > BTRFS_MAX_XATTR_SIZE(root->fs_info)) > + return -ENOSPC; > Besides making it silent, how about adding a ASSERT to cry out? (Although currently we'd never come into this case.) Reviewed-by: Liu Bo <bo.li.liu@oracle.com> Thanks, -liubo > key.objectid = objectid; > key.type = BTRFS_XATTR_ITEM_KEY; > -- > 2.10.1 > > -- > To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in > the body of a message to majordomo@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html -- To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Tue, Feb 21, 2017 at 09:39:05PM -0800, Liu Bo wrote: > On Mon, Feb 20, 2017 at 07:25:06PM +0100, David Sterba wrote: > > The space check in btrfs_insert_xattr_item is duplicated in it's caller > > (do_setxattr) so we won't hit the BUG_ON. Continuing without any check > > could be disasterous so turn it to a proper error handling. > > > > Signed-off-by: David Sterba <dsterba@suse.com> > > --- > > fs/btrfs/dir-item.c | 3 ++- > > 1 file changed, 2 insertions(+), 1 deletion(-) > > > > diff --git a/fs/btrfs/dir-item.c b/fs/btrfs/dir-item.c > > index 724504a2d7ac..640801082533 100644 > > --- a/fs/btrfs/dir-item.c > > +++ b/fs/btrfs/dir-item.c > > @@ -80,7 +80,8 @@ int btrfs_insert_xattr_item(struct btrfs_trans_handle *trans, > > struct extent_buffer *leaf; > > u32 data_size; > > > > - BUG_ON(name_len + data_len > BTRFS_MAX_XATTR_SIZE(root->fs_info)); > > + if (name_len + data_len > BTRFS_MAX_XATTR_SIZE(root->fs_info)) > > + return -ENOSPC; > > > > Besides making it silent, how about adding a ASSERT to cry out? > (Although currently we'd never come into this case.) I don't think we need the assert, the caller is supposed to handle the error. In this case it's validation of input parameters, that could possibly happen as the function is not static. -- To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/fs/btrfs/dir-item.c b/fs/btrfs/dir-item.c index 724504a2d7ac..640801082533 100644 --- a/fs/btrfs/dir-item.c +++ b/fs/btrfs/dir-item.c @@ -80,7 +80,8 @@ int btrfs_insert_xattr_item(struct btrfs_trans_handle *trans, struct extent_buffer *leaf; u32 data_size; - BUG_ON(name_len + data_len > BTRFS_MAX_XATTR_SIZE(root->fs_info)); + if (name_len + data_len > BTRFS_MAX_XATTR_SIZE(root->fs_info)) + return -ENOSPC; key.objectid = objectid; key.type = BTRFS_XATTR_ITEM_KEY;
The space check in btrfs_insert_xattr_item is duplicated in it's caller (do_setxattr) so we won't hit the BUG_ON. Continuing without any check could be disasterous so turn it to a proper error handling. Signed-off-by: David Sterba <dsterba@suse.com> --- fs/btrfs/dir-item.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-)