* [PATCH] ceph: fix incorrectly assigning random values to peer's members
@ 2022-06-06 7:28 Xiubo Li
2022-06-06 10:13 ` Luís Henriques
2022-06-06 10:23 ` Jeff Layton
0 siblings, 2 replies; 4+ messages in thread
From: Xiubo Li @ 2022-06-06 7:28 UTC (permalink / raw)
To: jlayton, idryomov; +Cc: vshankar, ceph-devel, Xiubo Li
For export the peer is empty in ceph.
URL: https://tracker.ceph.com/issues/55857
Signed-off-by: Xiubo Li <xiubli@redhat.com>
---
fs/ceph/caps.c | 15 +++++----------
1 file changed, 5 insertions(+), 10 deletions(-)
diff --git a/fs/ceph/caps.c b/fs/ceph/caps.c
index 0a48bf829671..8efa46ff4282 100644
--- a/fs/ceph/caps.c
+++ b/fs/ceph/caps.c
@@ -4127,16 +4127,11 @@ void ceph_handle_caps(struct ceph_mds_session *session,
p += flock_len;
}
- if (msg_version >= 3) {
- if (op == CEPH_CAP_OP_IMPORT) {
- if (p + sizeof(*peer) > end)
- goto bad;
- peer = p;
- p += sizeof(*peer);
- } else if (op == CEPH_CAP_OP_EXPORT) {
- /* recorded in unused fields */
- peer = (void *)&h->size;
- }
+ if (msg_version >= 3 && op == CEPH_CAP_OP_IMPORT) {
+ if (p + sizeof(*peer) > end)
+ goto bad;
+ peer = p;
+ p += sizeof(*peer);
}
if (msg_version >= 4) {
--
2.36.0.rc1
^ permalink raw reply related [flat|nested] 4+ messages in thread
* Re: [PATCH] ceph: fix incorrectly assigning random values to peer's members
2022-06-06 7:28 [PATCH] ceph: fix incorrectly assigning random values to peer's members Xiubo Li
@ 2022-06-06 10:13 ` Luís Henriques
2022-06-06 10:23 ` Jeff Layton
1 sibling, 0 replies; 4+ messages in thread
From: Luís Henriques @ 2022-06-06 10:13 UTC (permalink / raw)
To: Xiubo Li; +Cc: jlayton, idryomov, vshankar, ceph-devel
Xiubo Li <xiubli@redhat.com> writes:
> For export the peer is empty in ceph.
>
> URL: https://tracker.ceph.com/issues/55857
> Signed-off-by: Xiubo Li <xiubli@redhat.com>
> ---
> fs/ceph/caps.c | 15 +++++----------
> 1 file changed, 5 insertions(+), 10 deletions(-)
>
> diff --git a/fs/ceph/caps.c b/fs/ceph/caps.c
> index 0a48bf829671..8efa46ff4282 100644
> --- a/fs/ceph/caps.c
> +++ b/fs/ceph/caps.c
> @@ -4127,16 +4127,11 @@ void ceph_handle_caps(struct ceph_mds_session *session,
> p += flock_len;
> }
>
> - if (msg_version >= 3) {
> - if (op == CEPH_CAP_OP_IMPORT) {
> - if (p + sizeof(*peer) > end)
> - goto bad;
> - peer = p;
> - p += sizeof(*peer);
> - } else if (op == CEPH_CAP_OP_EXPORT) {
> - /* recorded in unused fields */
> - peer = (void *)&h->size;
> - }
> + if (msg_version >= 3 && op == CEPH_CAP_OP_IMPORT) {
> + if (p + sizeof(*peer) > end)
> + goto bad;
> + peer = p;
> + p += sizeof(*peer);
> }
>
> if (msg_version >= 4) {
> --
>
> 2.36.0.rc1
>
Are you sure this isn't breaking anything? Looking at MClientCaps.h, it
seems to be doing something similar, i.e. for the CAP_OP_EXPORT, the
'peer' is encoded where the 'size', 'max_size', 'truncate_size',... are
for the CAP_OP_IMPORT. This is definitely confusing and messy, but not
sure if your change isn't breaking something.
Cheers,
--
Luís
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] ceph: fix incorrectly assigning random values to peer's members
2022-06-06 7:28 [PATCH] ceph: fix incorrectly assigning random values to peer's members Xiubo Li
2022-06-06 10:13 ` Luís Henriques
@ 2022-06-06 10:23 ` Jeff Layton
2022-06-06 11:48 ` Xiubo Li
1 sibling, 1 reply; 4+ messages in thread
From: Jeff Layton @ 2022-06-06 10:23 UTC (permalink / raw)
To: Xiubo Li, idryomov; +Cc: vshankar, ceph-devel, ukernel
On Mon, 2022-06-06 at 15:28 +0800, Xiubo Li wrote:
> For export the peer is empty in ceph.
>
> URL: https://tracker.ceph.com/issues/55857
> Signed-off-by: Xiubo Li <xiubli@redhat.com>
> ---
> fs/ceph/caps.c | 15 +++++----------
> 1 file changed, 5 insertions(+), 10 deletions(-)
>
> diff --git a/fs/ceph/caps.c b/fs/ceph/caps.c
> index 0a48bf829671..8efa46ff4282 100644
> --- a/fs/ceph/caps.c
> +++ b/fs/ceph/caps.c
> @@ -4127,16 +4127,11 @@ void ceph_handle_caps(struct ceph_mds_session *session,
> p += flock_len;
> }
>
> - if (msg_version >= 3) {
> - if (op == CEPH_CAP_OP_IMPORT) {
> - if (p + sizeof(*peer) > end)
> - goto bad;
> - peer = p;
> - p += sizeof(*peer);
> - } else if (op == CEPH_CAP_OP_EXPORT) {
> - /* recorded in unused fields */
> - peer = (void *)&h->size;
> - }
> + if (msg_version >= 3 && op == CEPH_CAP_OP_IMPORT) {
> + if (p + sizeof(*peer) > end)
> + goto bad;
> + peer = p;
> + p += sizeof(*peer);
> }
>
> if (msg_version >= 4) {
This was added in commit 11df2dfb61 (ceph: add imported caps when
handling cap export message). If peer should always be NULL on an
export, I wonder what he was thinking by adding this in the first place?
Zheng, could you take a look here?
If this does turn out to be correct, then I think there is some further
cleanup you can do here, as you should be able to drop the peer argument
from handle_cap_export. That should also collapse some of the code down
in that function as well since lot of the target fields end up being 0s.
--
Jeff Layton <jlayton@kernel.org>
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] ceph: fix incorrectly assigning random values to peer's members
2022-06-06 10:23 ` Jeff Layton
@ 2022-06-06 11:48 ` Xiubo Li
0 siblings, 0 replies; 4+ messages in thread
From: Xiubo Li @ 2022-06-06 11:48 UTC (permalink / raw)
To: Jeff Layton, idryomov; +Cc: vshankar, ceph-devel, ukernel
On 6/6/22 6:23 PM, Jeff Layton wrote:
> On Mon, 2022-06-06 at 15:28 +0800, Xiubo Li wrote:
>> For export the peer is empty in ceph.
>>
>> URL: https://tracker.ceph.com/issues/55857
>> Signed-off-by: Xiubo Li <xiubli@redhat.com>
>> ---
>> fs/ceph/caps.c | 15 +++++----------
>> 1 file changed, 5 insertions(+), 10 deletions(-)
>>
>> diff --git a/fs/ceph/caps.c b/fs/ceph/caps.c
>> index 0a48bf829671..8efa46ff4282 100644
>> --- a/fs/ceph/caps.c
>> +++ b/fs/ceph/caps.c
>> @@ -4127,16 +4127,11 @@ void ceph_handle_caps(struct ceph_mds_session *session,
>> p += flock_len;
>> }
>>
>> - if (msg_version >= 3) {
>> - if (op == CEPH_CAP_OP_IMPORT) {
>> - if (p + sizeof(*peer) > end)
>> - goto bad;
>> - peer = p;
>> - p += sizeof(*peer);
>> - } else if (op == CEPH_CAP_OP_EXPORT) {
>> - /* recorded in unused fields */
>> - peer = (void *)&h->size;
>> - }
>> + if (msg_version >= 3 && op == CEPH_CAP_OP_IMPORT) {
>> + if (p + sizeof(*peer) > end)
>> + goto bad;
>> + peer = p;
>> + p += sizeof(*peer);
>> }
>>
>> if (msg_version >= 4) {
> This was added in commit 11df2dfb61 (ceph: add imported caps when
> handling cap export message). If peer should always be NULL on an
> export, I wonder what he was thinking by adding this in the first place?
> Zheng, could you take a look here?
>
> If this does turn out to be correct, then I think there is some further
> cleanup you can do here, as you should be able to drop the peer argument
> from handle_cap_export. That should also collapse some of the code down
> in that function as well since lot of the target fields end up being 0s.
Okay, will drop this. The head structs are different in ceph and kernel.
-- Xiubo
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2022-06-06 11:49 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-06-06 7:28 [PATCH] ceph: fix incorrectly assigning random values to peer's members Xiubo Li
2022-06-06 10:13 ` Luís Henriques
2022-06-06 10:23 ` Jeff Layton
2022-06-06 11:48 ` Xiubo Li
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.