[PATCH] ctdb - add sock_packet_header structure and sock_client abstraction

[Amitay Isaacs]

On Fri, Sep 1, 2017 at 3:32 PM, Volker Lendecke <Volker.Lendecke at sernet.de>
wrote:

> On Fri, Sep 01, 2017 at 12:15:59PM +1000, Amitay Isaacs via
> samba-technical wrote:
> > +int sock_packet_header_pull(uint8_t *buf, size_t buflen,
> > +                         struct sock_packet_header *out, size_t *npull)
> > +{
> > +     size_t offset = 0, np;
> > +     int ret;
> > +
> > +     ret = ctdb_uint32_pull(buf+offset, buflen-offset, &out->length,
> &np);
> > +     if (ret != 0) {
> > +             return ret;
> > +     }
> > +     offset += np;
> > +
> > +     ret = ctdb_uint32_pull(buf+offset, buflen-offset, &out->reqid,
> &np);
> > +     if (ret != 0) {
> > +             return ret;
> > +     }
> > +     offset += np;
> > +
> > +     *npull = offset;
> > +     return 0;
> > +}
>
> I'm always a bit worried about pull functions without overflow checks.
> I know this is just an internal protocol, but even there I'd
> appreciate if we followed good practices and always check for overflow
> and buflen checks.
>
>
The overflow checks are done in individual smaller elements.  In this case
it is done in ctdb_uint32_pull().

Now the overflow checks are done consistently for each element of the
structure.  When the lengths are parsed from the buffer, the checks are
applied to those lengths in the higher-level pull function.  This was part
of the protocol marshalling clean up that just went upstream.

Amitay.