diff mbox series

[v2] ceph: force updating the msg pointer in non-split case

Message ID 20230518014723.148327-1-xiubli@redhat.com
State New
Headers show
Series [v2] ceph: force updating the msg pointer in non-split case | expand

Commit Message

Xiubo Li May 18, 2023, 1:47 a.m. UTC
From: Xiubo Li <xiubli@redhat.com>

When the MClientSnap reqeust's op is not CEPH_SNAP_OP_SPLIT the
request may still contain a list of 'split_realms', and we need
to skip it anyway. Or it will be parsed as a corrupt snaptrace.

Cc: stable@vger.kernel.org
Cc: Frank Schilder <frans@dtu.dk>
Reported-by: Frank Schilder <frans@dtu.dk>
URL: https://tracker.ceph.com/issues/61200
Signed-off-by: Xiubo Li <xiubli@redhat.com>
---

V2:
- Add a detail comment for the code.


 fs/ceph/snap.c | 13 +++++++++++++
 1 file changed, 13 insertions(+)

Comments

Ilya Dryomov May 18, 2023, 9:19 a.m. UTC | #1
On Thu, May 18, 2023 at 3:48 AM <xiubli@redhat.com> wrote:
>
> From: Xiubo Li <xiubli@redhat.com>
>
> When the MClientSnap reqeust's op is not CEPH_SNAP_OP_SPLIT the
> request may still contain a list of 'split_realms', and we need
> to skip it anyway. Or it will be parsed as a corrupt snaptrace.
>
> Cc: stable@vger.kernel.org
> Cc: Frank Schilder <frans@dtu.dk>
> Reported-by: Frank Schilder <frans@dtu.dk>
> URL: https://tracker.ceph.com/issues/61200
> Signed-off-by: Xiubo Li <xiubli@redhat.com>
> ---
>
> V2:
> - Add a detail comment for the code.
>
>
>  fs/ceph/snap.c | 13 +++++++++++++
>  1 file changed, 13 insertions(+)
>
> diff --git a/fs/ceph/snap.c b/fs/ceph/snap.c
> index 0e59e95a96d9..0f00f977c0f0 100644
> --- a/fs/ceph/snap.c
> +++ b/fs/ceph/snap.c
> @@ -1114,6 +1114,19 @@ void ceph_handle_snap(struct ceph_mds_client *mdsc,
>                                 continue;
>                         adjust_snap_realm_parent(mdsc, child, realm->ino);
>                 }
> +       } else {
> +               /*
> +                * In non-SPLIT op case both the 'num_split_inos' and
> +                * 'num_split_realms' should always be 0 and this will
> +                * do nothing. But the MDS has one bug that in one of
> +                * the UPDATE op cases it will pass a 'split_realms'
> +                * list by mistake, and then will corrupted the snap
> +                * trace in ceph_update_snap_trace().
> +                *
> +                * So we should skip them anyway here.
> +                */
> +               p += sizeof(u64) * num_split_inos;
> +               p += sizeof(u64) * num_split_realms;
>         }
>
>         /*
> --
> 2.40.1
>

LGTM, staged for 6.4-rc3 with a slightly amended comment.

Thanks,

                Ilya
Xiubo Li May 18, 2023, 9:36 a.m. UTC | #2
On 5/18/23 17:19, Ilya Dryomov wrote:
> On Thu, May 18, 2023 at 3:48 AM <xiubli@redhat.com> wrote:
>> From: Xiubo Li <xiubli@redhat.com>
>>
>> When the MClientSnap reqeust's op is not CEPH_SNAP_OP_SPLIT the
>> request may still contain a list of 'split_realms', and we need
>> to skip it anyway. Or it will be parsed as a corrupt snaptrace.
>>
>> Cc: stable@vger.kernel.org
>> Cc: Frank Schilder <frans@dtu.dk>
>> Reported-by: Frank Schilder <frans@dtu.dk>
>> URL: https://tracker.ceph.com/issues/61200
>> Signed-off-by: Xiubo Li <xiubli@redhat.com>
>> ---
>>
>> V2:
>> - Add a detail comment for the code.
>>
>>
>>   fs/ceph/snap.c | 13 +++++++++++++
>>   1 file changed, 13 insertions(+)
>>
>> diff --git a/fs/ceph/snap.c b/fs/ceph/snap.c
>> index 0e59e95a96d9..0f00f977c0f0 100644
>> --- a/fs/ceph/snap.c
>> +++ b/fs/ceph/snap.c
>> @@ -1114,6 +1114,19 @@ void ceph_handle_snap(struct ceph_mds_client *mdsc,
>>                                  continue;
>>                          adjust_snap_realm_parent(mdsc, child, realm->ino);
>>                  }
>> +       } else {
>> +               /*
>> +                * In non-SPLIT op case both the 'num_split_inos' and
>> +                * 'num_split_realms' should always be 0 and this will
>> +                * do nothing. But the MDS has one bug that in one of
>> +                * the UPDATE op cases it will pass a 'split_realms'
>> +                * list by mistake, and then will corrupted the snap
>> +                * trace in ceph_update_snap_trace().
>> +                *
>> +                * So we should skip them anyway here.
>> +                */
>> +               p += sizeof(u64) * num_split_inos;
>> +               p += sizeof(u64) * num_split_realms;
>>          }
>>
>>          /*
>> --
>> 2.40.1
>>
> LGTM, staged for 6.4-rc3 with a slightly amended comment.

Sure, thanks.

- Xiubo


> Thanks,
>
>                  Ilya
>
diff mbox series

Patch

diff --git a/fs/ceph/snap.c b/fs/ceph/snap.c
index 0e59e95a96d9..0f00f977c0f0 100644
--- a/fs/ceph/snap.c
+++ b/fs/ceph/snap.c
@@ -1114,6 +1114,19 @@  void ceph_handle_snap(struct ceph_mds_client *mdsc,
 				continue;
 			adjust_snap_realm_parent(mdsc, child, realm->ino);
 		}
+	} else {
+		/*
+		 * In non-SPLIT op case both the 'num_split_inos' and
+		 * 'num_split_realms' should always be 0 and this will
+		 * do nothing. But the MDS has one bug that in one of
+		 * the UPDATE op cases it will pass a 'split_realms'
+		 * list by mistake, and then will corrupted the snap
+		 * trace in ceph_update_snap_trace().
+		 *
+		 * So we should skip them anyway here.
+		 */
+		p += sizeof(u64) * num_split_inos;
+		p += sizeof(u64) * num_split_realms;
 	}
 
 	/*