On 06/26/2018 04:39 PM, Ilya Dryomov wrote:
On Tue, Jun 26, 2018 at 10:17 AM Yan, Zheng <ukernel@xxxxxxxxx> wrote:
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.
If we add it to reserve(), what about append(), etc?
I applied that patch yesterday, but on a second thought, I think we
should revert it and change ceph_pagelist_encode_string() to take u32,
thus pushing the responsibility for that check into the callers.
I have already changed ceph_osdc_notify{,ack}() which took size_t to
u32 (see testing). Here size_t is mandated, but ceph_pre_init_acls()
can do that check before calling ceph_pagelist_reserve().
No need to patch pagelist.c.
How about introduce a helper ceph_pagelist_encode_size_t() to deal with
type size_t.
It seems a bit easier than checking in the caller.
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
