| Message ID | 20230518014723.148327-1-xiubli@redhat.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [v2] ceph: force updating the msg pointer in non-split case | expand |
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
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 --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; } /*