[PATCH] ctdb - add sock_packet_header structure and sock_client abstraction

[Volker Lendecke]

On Fri, Sep 01, 2017 at 03:47:38PM +1000, Amitay Isaacs via samba-technical wrote:
> 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.

Ok, that makes sense. It just looks worrying to me to have all these
integer calculations in pull functions without any visible overflow
checks. Thanks for the explanation. RB+ for this piece.

Volker

-- 
SerNet GmbH, Bahnhofsallee 1b, 37081 Göttingen
phone: +49-551-370000-0, fax: +49-551-370000-9
AG Göttingen, HRB 2816, GF: Dr. Johannes Loxen
http://www.sernet.de, mailto:kontakt at sernet.de