On Tue, Jun 26, 2018 at 2:44 PM cgxu519 <cgxu519@xxxxxxx> wrote: > > > On 06/26/2018 02:32 PM, Yan, Zheng wrote: > > > >> On Jun 26, 2018, at 13:04, cgxu519 <cgxu519@xxxxxxx> wrote: > >> > >> Hi Ilya, > >> > >> Any objection for this? Maybe I should fix the overlapping return code as well. > >> Should I split the patch to libceph and ceph part? > >> > >> On 06/24/2018 09:06 PM, Chengguang Xu wrote: > >>> Using ceph_pagelist_encode_string() instead of combination of > >>> ceph_pagelist_encode_32() and ceph_pagelist_append() when encoding > >>> string. Meanwhile add return code check for ceph_pagelist_encode_string(). > >>> > >>> Signed-off-by: Chengguang Xu <cgxu519@xxxxxxx> > >>> --- > >>> v2: > >>> - Add return code check for ceph_pagelist_encode_string(). > >>> > >>> fs/ceph/acl.c | 18 ++++++++++++------ > >>> net/ceph/osd_client.c | 16 +++++++++------- > >>> 2 files changed, 21 insertions(+), 13 deletions(-) > >>> > >>> diff --git a/fs/ceph/acl.c b/fs/ceph/acl.c > >>> index 5da752751a2b..6f0210984456 100644 > >>> --- a/fs/ceph/acl.c > >>> +++ b/fs/ceph/acl.c > >>> @@ -222,14 +222,17 @@ int ceph_pre_init_acls(struct inode *dir, umode_t *mode, > >>> err = ceph_pagelist_reserve(pagelist, len + val_size1 + 8); > >>> if (err) > >>> goto out_err; > >>> - ceph_pagelist_encode_string(pagelist, XATTR_NAME_POSIX_ACL_ACCESS, > >>> - len); > >>> + err = ceph_pagelist_encode_string(pagelist, > >>> + XATTR_NAME_POSIX_ACL_ACCESS, len); > >>> + if (err) > >>> + goto out_err; > >>> err = posix_acl_to_xattr(&init_user_ns, acl, > >>> tmp_buf, val_size1); > >>> if (err < 0) > >>> goto out_err; > >>> - ceph_pagelist_encode_32(pagelist, val_size1); > >>> - ceph_pagelist_append(pagelist, tmp_buf, val_size1); > >>> + err = ceph_pagelist_encode_string(pagelist, tmp_buf, val_size1); > >>> + if (err) > >>> + goto out_err; > >>> } > > the code first reserve space, then add data to page list. no need to check return > > Hi Yan, > > Thanks for your reply, after applying below patch > ceph_pagelist_encode_string() > may return error(maybe very rare case?) even we have reserved space > before calling it. > > [PATCH v2 1/2] ceph: check string length in > ceph_pagelist_encode_string() for safety > should make ceph_pagelist_reserve() do the same check. If reservation succeeds, ceph_pagelist_encode_string() should never fail. Regards Yan, Zheng > Thanks, > Chengguang. > > > -- > To unsubscribe from this list: send the line "unsubscribe ceph-devel" in > the body of a message to majordomo@xxxxxxxxxxxxxxx > More majordomo info at http://vger.kernel.org/majordomo-info.html -- To unsubscribe from this list: send the line "unsubscribe ceph-devel" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
