| Message ID | 20231208043305.91249-1-xiubli@redhat.com |
|---|---|
| Headers | show |
| Series | libceph: fix sparse-read failure bug | expand |
On Fri, 2023-12-08 at 12:33 +0800, xiubli@redhat.com wrote: > From: Xiubo Li <xiubli@redhat.com> > > Once this happens that means there have bugs. > > URL: https://tracker.ceph.com/issues/63586 > Signed-off-by: Xiubo Li <xiubli@redhat.com> > --- > net/ceph/osd_client.c | 4 +++- > 1 file changed, 3 insertions(+), 1 deletion(-) > > diff --git a/net/ceph/osd_client.c b/net/ceph/osd_client.c > index 5753036d1957..848ef19055a0 100644 > --- a/net/ceph/osd_client.c > +++ b/net/ceph/osd_client.c > @@ -5912,10 +5912,12 @@ static int osd_sparse_read(struct ceph_connection *con, > fallthrough; > case CEPH_SPARSE_READ_DATA: > if (sr->sr_index >= count) { > - if (sr->sr_datalen && count) > + if (sr->sr_datalen) { > pr_warn_ratelimited("sr_datalen %u sr_index %d count %u\n", > sr->sr_datalen, sr->sr_index, > count); > + return -EREMOTEIO; > + } > > sr->sr_state = CEPH_SPARSE_READ_HDR; > goto next_op; Nice catch. You can add: Reviewed-by: Jeff Layton <jlayton@kernel.org>
On 12/8/23 19:31, Ilya Dryomov wrote: > On Fri, Dec 8, 2023 at 5:34 AM <xiubli@redhat.com> wrote: >> From: Xiubo Li <xiubli@redhat.com> >> >> Once this happens that means there have bugs. >> >> URL: https://tracker.ceph.com/issues/63586 >> Signed-off-by: Xiubo Li <xiubli@redhat.com> >> --- >> net/ceph/osd_client.c | 4 +++- >> 1 file changed, 3 insertions(+), 1 deletion(-) >> >> diff --git a/net/ceph/osd_client.c b/net/ceph/osd_client.c >> index 5753036d1957..848ef19055a0 100644 >> --- a/net/ceph/osd_client.c >> +++ b/net/ceph/osd_client.c >> @@ -5912,10 +5912,12 @@ static int osd_sparse_read(struct ceph_connection *con, >> fallthrough; >> case CEPH_SPARSE_READ_DATA: >> if (sr->sr_index >= count) { >> - if (sr->sr_datalen && count) >> + if (sr->sr_datalen) { >> pr_warn_ratelimited("sr_datalen %u sr_index %d count %u\n", >> sr->sr_datalen, sr->sr_index, >> count); >> + return -EREMOTEIO; >> + } >> >> sr->sr_state = CEPH_SPARSE_READ_HDR; >> goto next_op; >> -- >> 2.43.0 >> > Hi Xiubo, > > There is a patch in linux-next, also from you, which is conflicting > with this one: cca19d307d35 ("libceph: check the data length when > sparse read finishes"). Do you want it replaced? Yeah, let me fold them. Thanks Jeff and Ilya. > Thanks, > > Ilya >
On 12/8/23 23:28, Ilya Dryomov wrote: > On Fri, Dec 8, 2023 at 4:18 PM Xiubo Li <xiubli@redhat.com> wrote: >> >> On 12/8/23 19:31, Ilya Dryomov wrote: >> >> On Fri, Dec 8, 2023 at 5:34 AM <xiubli@redhat.com> wrote: >> >> From: Xiubo Li <xiubli@redhat.com> >> >> Once this happens that means there have bugs. >> >> URL: https://tracker.ceph.com/issues/63586 >> Signed-off-by: Xiubo Li <xiubli@redhat.com> >> --- >> net/ceph/osd_client.c | 4 +++- >> 1 file changed, 3 insertions(+), 1 deletion(-) >> >> diff --git a/net/ceph/osd_client.c b/net/ceph/osd_client.c >> index 5753036d1957..848ef19055a0 100644 >> --- a/net/ceph/osd_client.c >> +++ b/net/ceph/osd_client.c >> @@ -5912,10 +5912,12 @@ static int osd_sparse_read(struct ceph_connection *con, >> fallthrough; >> case CEPH_SPARSE_READ_DATA: >> if (sr->sr_index >= count) { >> - if (sr->sr_datalen && count) >> + if (sr->sr_datalen) { >> pr_warn_ratelimited("sr_datalen %u sr_index %d count %u\n", >> sr->sr_datalen, sr->sr_index, >> count); >> + return -EREMOTEIO; >> + } >> >> sr->sr_state = CEPH_SPARSE_READ_HDR; >> goto next_op; >> -- >> 2.43.0 >> >> Hi Xiubo, >> >> There is a patch in linux-next, also from you, which is conflicting >> with this one: cca19d307d35 ("libceph: check the data length when >> sparse read finishes"). Do you want it replaced? >> >> Ilya, >> >> I found the commit cca19d307d35 has already in the master branch. Could you fold and update it ? > I would like to see the entire fix first. You seem to be going back > and forth between just issuing a warning or also returning an error and > the precise if condition there, so I'm starting to think that the bug > is not fully understood and neither patch might be necessary. Sure, I just sent out the second version. Thanks - Xiubo > Thanks, > > Ilya >
From: Xiubo Li <xiubli@redhat.com> Xiubo Li (2): libceph: fail the sparse-read if there still has data in socket libceph: just wait for more data to be available on the socket net/ceph/messenger_v1.c | 18 ++++++++++-------- net/ceph/osd_client.c | 4 +++- 2 files changed, 13 insertions(+), 9 deletions(-)