Re: test osd on zfs

[Brian Behlendorf <behlendorf1@llnl.gov>]

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

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:
         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