From patchwork Wed Apr 17 21:14:38 2013 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Brian Behlendorf X-Patchwork-Id: 2456581 Return-Path: X-Original-To: patchwork-ceph-devel@patchwork.kernel.org Delivered-To: patchwork-process-083081@patchwork2.kernel.org Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by patchwork2.kernel.org (Postfix) with ESMTP id 34AA1DF23A for ; Wed, 17 Apr 2013 21:15:00 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S965485Ab3DQVO7 (ORCPT ); Wed, 17 Apr 2013 17:14:59 -0400 Received: from prdiron-3.llnl.gov ([128.15.143.173]:56146 "EHLO prdiron-3.llnl.gov" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S965436Ab3DQVO6 (ORCPT ); Wed, 17 Apr 2013 17:14:58 -0400 X-Attachments: Received: from eris.llnl.gov (HELO [192.168.1.129]) ([128.115.7.7]) by prdiron-3.llnl.gov with ESMTP; 17 Apr 2013 14:14:42 -0700 Message-ID: <516F10BE.6020103@llnl.gov> Date: Wed, 17 Apr 2013 14:14:38 -0700 From: Brian Behlendorf User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:16.0) Gecko/20121011 Thunderbird/16.0.1 MIME-Version: 1.0 To: Mark Nelson CC: Stefan Priebe , Yehuda Sadeh , Sage Weil , Jeff Mitchell , Henry C Chang , Aleksey Leonov , ceph-devel Subject: Re: test osd on zfs References: <516E7D5C.7080309@nazarianin.com> <516ECFB6.8090107@gmail.com> <516EF07E.4000909@llnl.gov> <516EF34E.5000000@profihost.ag> <516F0321.2@inktank.com> In-Reply-To: <516F0321.2@inktank.com> Sender: ceph-devel-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: ceph-devel@vger.kernel.org On 04/17/2013 01:16 PM, Mark Nelson wrote: > I'll let Brian talk about the virtues of ZFS, I think the virtues of ZFS have been discussed at length in various other forums. But in short it brings some nice functionality to the table which may be useful to ceph and that's worth exploring. >>>> >>>> diff --git a/module/zfs/zpl_xattr.c b/module/zfs/zpl_xattr.c >>>> index c03764f..9f4d63c 100644 >>>> --- a/module/zfs/zpl_xattr.c >>>> +++ b/module/zfs/zpl_xattr.c >>>> @@ -225,6 +225,11 @@ zpl_xattr_get_dir(struct inode *ip, const char >>>> *name, >>>> void *value, >>>> goto out; >>>> } >>>> >>>> + if (size < i_size_read(xip)) { >>>> + error = -ERANGE; >>>> + goto out; >>>> + } >>>> + >>>> error = zpl_read_common(xip, value, size, 0, UIO_SYSSPACE, >>>> 0, cr); >>>> out: >>>> if (xip) >>>> @@ -263,7 +268,10 @@ zpl_xattr_get_sa(struct inode *ip, const char >>>> *name, >>>> void *value, size_t size) >>>> if (!size) >>>> return (nv_size); >>>> >>>> - memcpy(value, nv_value, MIN(size, nv_size)); >>>> >>>> + if (size < nv_size) >>>> + return (-ERANGE); >>> >>> Note, that zpl_xattr_get_sa() is called by __zpl_xattr_get() which can >>> also be called by zpl_xattr_get() to test for xattr existence. So it >>> needs to make sure that zpl_xattr_set() doesn't fail if getting >>> -ERANGE. This shouldn't be a problem. The zpl_xattr_get() call from zpl_xattr_set() passes a NULL value and zero size which will prevent it from hitting the ERANGE error. It will return instead the xattr size as expected. >>> >>>> + >>>> + memcpy(value, nv_value, size); >>>> >>>> return (MIN(size, nv_size)); >>> >>> No need for MIN() here. Thanks for catching that. I've opened a pull request at github with the updated fix and kicked it off for automated testing. It would be nice to verify this resolves the crash. https://github.com/zfsonlinux/zfs/pull/1409 if (xip) @@ -263,9 +268,12 @@ zpl_xattr_get_sa(struct inode *ip, const char *name, void * if (!size) return (nv_size); - memcpy(value, nv_value, MIN(size, nv_size)); + if (size < nv_size) + return (-ERANGE); + + memcpy(value, nv_value, size); - return (MIN(size, nv_size)); + return (size); } static int --- To unsubscribe from this list: send the line "unsubscribe ceph-devel" 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/module/zfs/zpl_xattr.c b/module/zfs/zpl_xattr.c index c03764f..42a06ad 100644 --- a/module/zfs/zpl_xattr.c +++ b/module/zfs/zpl_xattr.c @@ -225,6 +225,11 @@ zpl_xattr_get_dir(struct inode *ip, const char *name, void goto out; } + if (size < i_size_read(xip)) { + error = -ERANGE; + goto out; + } + error = zpl_read_common(xip, value, size, 0, UIO_SYSSPACE, 0, cr); out: