[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