[RFC] Performance improvements

Swen Schillig swen at vnet.ibm.com
Wed Jun 20 09:00:56 UTC 2018


On Tue, 2018-06-19 at 11:53 -0700, Jeremy Allison wrote:
> On Mon, Jun 18, 2018 at 12:57:14PM +0200, Swen Schillig via samba-
> technical wrote:
> > On Mon, 2018-06-18 at 10:58 +0200, Andreas Schneider wrote:
> > > On Monday, 18 June 2018 10:04:03 CEST Swen Schillig via samba-
> > > technical wrote:
> > > 
> > > There are some nice cleanups.
> > 
> > Thanks.
> > 
> > > 
> > > In the first patch:
> > > 
> > > +	int num_ready;
> > > 
> > > +		int new_size = num_ready + queue->buffer.length;
> > > 
> > > Please use size_t.
> > 
> > Done.
> > 
> > Won't send the updated patch series yet...
> > expecting more comments / updates soon.
> 
> Don't have a lot of bandwidth right now, but can you
> check all the packet + offset calculations are integer
> wrap safe (yes I know they're updates of existing
> code, but it's nice to add safety when stuff gets
> updated).
> 
> Jeremy.
> 
Ok, what do you thin of such an addition to the patchset ?

diff --git a/ctdb/common/ctdb_io.c b/ctdb/common/ctdb_io.c
index c0a432fd5d0..3f0efd4f181 100644
--- a/ctdb/common/ctdb_io.c
+++ b/ctdb/common/ctdb_io.c
@@ -118,6 +118,13 @@ process_pkt:
        queue->buffer.offset += pkt_size;
        queue->buffer.length -= pkt_size;
 
+       if (queue->buffer.offset < pkt_size ||
+           queue->buffer.offset > queue->buffer.size) {
+               D_ERR("buffer offset overflow\n");
+               queue->callback(NULL, 0, queue->private_data);
+               return;
+       }
+
        if ((queue->buffer.length == 0) &&
            (queue->buffer.size > queue->buffer_size)) {
                /* if the buffer is empty and more than its
@@ -175,6 +182,17 @@ static void queue_io_read(struct ctdb_queue *queue)
 
                size_t new_size = MAX(num_ready + qb->length + qb->offset,
                                   queue->buffer_size);
+               if (new_size < num_ready) {
+                       /* with qb->length and qb->offset both being guaranteed
+                        * to be smaller than MAX_TALLOC_SIZE which in turn is
+                        * only a fraction of MAX_INT, the only possibility for
+                        * new_size being smaller than num_ready is an overflow
+                        */
+                       D_ERR("read error num_ready overflow %u\n", num_ready);
+                       queue->callback(NULL, 0, queue->private_data);
+                       return;
+               }
+
                qb->data = talloc_realloc_size(queue, qb->data, new_size);
                qb->size = new_size;




More information about the samba-technical mailing list