Re: [PATCH] rbd: fix response length parameter for rbd_obj_method_sync()

[Dongsheng Yang <dongsheng.yang@xxxxxxxxxxxx>]

On 08/09/2019 03:34 PM, Ilya Dryomov wrote:
> On Fri, Aug 9, 2019 at 9:05 AM Dongsheng Yang
> <dongsheng.yang@xxxxxxxxxxxx> wrote:
> > Response will be an encoded string, which includes a length.
> > So we need to allocate more buf of sizeof (__le32) in reply
> > buffer, and pass the reply buffer size as a parameter in
> > rbd_obj_method_sync function.
> >
> > Signed-off-by: Dongsheng Yang <dongsheng.yang@xxxxxxxxxxxx>
> > ---
> >   drivers/block/rbd.c | 9 ++++++---
> >   1 file changed, 6 insertions(+), 3 deletions(-)
> >
> > diff --git a/drivers/block/rbd.c b/drivers/block/rbd.c
> > index 3327192..db55ece 100644
> > --- a/drivers/block/rbd.c
> > +++ b/drivers/block/rbd.c
> > @@ -5661,14 +5661,17 @@ static int rbd_dev_v2_object_prefix(struct rbd_device *rbd_dev)
> >          void *reply_buf;
> >          int ret;
> >          void *p;
> > +       size_t size;
> >
> > -       reply_buf = kzalloc(RBD_OBJ_PREFIX_LEN_MAX, GFP_KERNEL);
> > +       /* Response will be an encoded string, which includes a length */
> > +       size = sizeof (__le32) + RBD_OBJ_PREFIX_LEN_MAX;
> > +       reply_buf = kzalloc(size, GFP_KERNEL);
> >          if (!reply_buf)
> >                  return -ENOMEM;
> >
> >          ret = rbd_obj_method_sync(rbd_dev, &rbd_dev->header_oid,
> >                                    &rbd_dev->header_oloc, "get_object_prefix",
> > -                                 NULL, 0, reply_buf, RBD_OBJ_PREFIX_LEN_MAX);
> > +                                 NULL, 0, reply_buf, size);
> >          dout("%s: rbd_obj_method_sync returned %d\n", __func__, ret);
> >          if (ret < 0)
> >                  goto out;
> > @@ -6697,7 +6700,7 @@ static int rbd_dev_image_id(struct rbd_device *rbd_dev)
> >
> >          ret = rbd_obj_method_sync(rbd_dev, &oid, &rbd_dev->header_oloc,
> >                                    "get_id", NULL, 0,
> > -                                 response, RBD_IMAGE_ID_LEN_MAX);
> > +                                 response, size);
> >          dout("%s: rbd_obj_method_sync returned %d\n", __func__, ret);
> >          if (ret == -ENOENT) {
> >                  image_id = kstrdup("", GFP_KERNEL);
> Hi Dongsheng,
>
> AFAIR RBD_OBJ_PREFIX_LEN_MAX and RBD_IMAGE_ID_LEN_MAX are arbitrary
> constants with enough slack for length, etc.  We shouldn't ever see
> object prefixes or image ids that are longer than 62 bytes.

Hi Ilya,
    Yes, this patch is not fixing a real problem, as you mentioned we 
dont have
prefixes or image ids longer than 62 bytes. But this patch is going to 
make it
logical consistent.

The most confusing case is for rbd_dev_image_id(), size of response is 
already
RBD_IMAGE_ID_LEN_MAX + sizeof (__le32). but the @resp_length in 
rbd_obj_method_sync()
is still RBD_IMAGE_ID_LEN_MAX.

Thanx
> Thanks,
>
>                  Ilya