[PATCH] Fixes to CTDB protocol handling

Martin Schwenke martin at meltin.net
Thu May 12 05:37:09 UTC 2016


On Tue, 10 May 2016 19:28:58 +1000, Amitay Isaacs <amitay at gmail.com>
wrote:

> On Wed, May 4, 2016 at 1:30 AM, Volker Lendecke <Volker.Lendecke at sernet.de>
> wrote:
> 
> > On Tue, May 03, 2016 at 04:49:57PM +1000, Amitay Isaacs wrote:  
> > > Here are some more fixes as suggested by Volker.  Add checks before using
> > > data on wire and checks integer wrapping.  
> >
> > Quick remark: I believe
> >
> > +       if (wire->num * sizeof(struct ctdb_statistics) > buflen) {
> > +               return EMSGSIZE;
> > +       }
> >
> > is wrong. The multiplication could overflow leading to the
> > check succeeding where it should not. I think that something
> > like
> >
> > if (wire->num > buflen / sizeof(struct ..)
> >
> > or so should avoid that.
> >
> > Then there's a
> >
> >         n = offsetof(struct ctdb_rec_data_wire, data) +
> >                 wire->keylen + wire->datalen;
> >
> > for which I'm uncertain about the overflow in the +.
> >
> > I know, C network protocol code is almost impossible to get right :-(
> >  
> 
> Here's another attempt at checking integer overflow in protocol
> marshaling.  All the changes are in the second patch.

Reviewed-by: Martin Schwenke <martin at meltin.net>

However, as discussed, we should obviously wait until Volker takes a
look before pushing...

peace & happiness,
martin



More information about the samba-technical mailing list