Hi,
Le dimanche 25 janvier 2015 à 17:29 +0200, Haggai Eran a écrit :
> On 22/01/2015 15:28, Yann Droneaud wrote:
> > Instead of silently truncating extended QUERY_DEVICE uverb's
> > response, see commit 5a77abf9a97a ("IB/core: Add support for
> > extended query device caps")), this patch makes function
> > ib_uverbs_ex_query_device() check the available space in the
> > response buffer against the requested features set in comp_mask
> > (currently only IB_USER_VERBS_EX_QUERY_DEVICE_ODP per commit
> > 860f10a799c8 ("IB/core: Add flags for on demand paging support").
>
> Currently you only need to check two possible sizes. With each
> additional field you will need to check another size.
Yes, that's mandatory for any interface with a dynamic, ever growing,
data structure.
> Sooner or later someone will forget to add a check and will create a
> non-backward
> compatible kernel.
>
That's why we have to review userspace interface patch with lot of care.
Anyway, such a change should be fixed easily as it's would be
definitively a bug.
If the behavior I'm proposing is in place, userspace won't be allowed
to supply unsupported bits in a request, so older userspace requests on
newer kernels won't trigger unsupported new behavior. This ensure we can
extend the interfance with no risk of breaking existing program.
> > If the response buffer is not large enough to store the expected
> > response, -ENOSPC is returned to userspace so that it can adjust
> > the size of its buffer.
> >
> > Note: as offsetof() is used to retrieve the size of the lower chunk
> > of the response, beware that it only works if the upper chunk
> > is right after, without any implicit padding. And, as the size of
> > the latter chunk is added to the base size, implicit padding at the
> > end of the structure is not taken in account. Both point must be
> > taken in account when extending the uverbs functionalities.
>
> Another point future contributors will easily miss.
>
One should use "pahole" tool when modifying data structure part of the
ABI to ensure the data structure are correctly aligned on various
architectures.
> > Link: http://mid.gmane.org/[email protected]
> > Cc: Sagi Grimberg <[email protected]>
> > Cc: Shachar Raindel <[email protected]>
> > Cc: Eli Cohen <[email protected]>
> > Cc: Haggai Eran <[email protected]>
> > Signed-off-by: Yann Droneaud <[email protected]>
> > ---
> > drivers/infiniband/core/uverbs_cmd.c | 13 ++++++++++++-
> > 1 file changed, 12 insertions(+), 1 deletion(-)
> >
> > diff --git a/drivers/infiniband/core/uverbs_cmd.c
> > b/drivers/infiniband/core/uverbs_cmd.c
> > index 532d8eba8b02..8668b328b7e6 100644
> > --- a/drivers/infiniband/core/uverbs_cmd.c
> > +++ b/drivers/infiniband/core/uverbs_cmd.c
> > @@ -3302,6 +3302,7 @@ int ib_uverbs_ex_query_device(struct ib_uverbs_file
> > *file,
> > struct ib_uverbs_ex_query_device cmd;
> > struct ib_device_attr attr;
> > struct ib_device *device;
> > + size_t resp_len;
> > int err;
> >
> > device = file->device->ib_dev;
> > @@ -3315,6 +3316,11 @@ int ib_uverbs_ex_query_device(struct ib_uverbs_file
> > *file,
> > if (cmd.reserved)
> > return -EINVAL;
> >
> > + resp_len = offsetof(typeof(resp), odp_caps);
> > +
> > + if (ucore->outlen < resp_len)
> > + return -ENOSPC;
> > +
> > err = device->query_device(device, &attr);
> > if (err)
> > return err;
> > @@ -3325,6 +3331,11 @@ int ib_uverbs_ex_query_device(struct ib_uverbs_file
> > *file,
> >
> > #ifdef CONFIG_INFINIBAND_ON_DEMAND_PAGING
> > if (cmd.comp_mask & IB_USER_VERBS_EX_QUERY_DEVICE_ODP) {
> > + resp_len += sizeof(resp.odp_caps);
> > +
> > + if (ucore->outlen < resp_len)
> > + return -ENOSPC;
> > +
> > resp.odp_caps.general_caps = attr.odp_caps.general_caps;
> > resp.odp_caps.per_transport_caps.rc_odp_caps =
> > attr.odp_caps.per_transport_caps.rc_odp_caps;
> > @@ -3336,7 +3347,7 @@ int ib_uverbs_ex_query_device(struct ib_uverbs_file
> > *file,
> > }
> > #endif
> >
> > - err = ib_copy_to_udata(ucore, &resp, sizeof(resp));
> > + err = ib_copy_to_udata(ucore, &resp, resp_len);
> > if (err)
> > return err;
> >
> >
>
Regards.
--
Yann Droneaud
OPTEYA
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html
