On Wed, Aug 28, 2013 at 09:49:39AM -0400, Simo Sorce wrote:
On Tue, 2013-08-27 at 10:54 +0200, Lukas Slebodnik wrote:
> On (27/08/13 10:42), Pavel Březina wrote:
> >https://fedorahosted.org/sssd/ticket/2059
>
> >From 573dcd8fc8a87bcfab61d0113c3bc3a49b81f95c Mon Sep 17 00:00:00 2001
> >From: =?UTF-8?q?Pavel=20B=C5=99ezina?= <pbrezina(a)redhat.com>
> >Date: Thu, 22 Aug 2013 14:38:54 +0200
> >Subject: [PATCH] sss_packet_grow: correctly pad packet length to 512B
> >
> >https://fedorahosted.org/sssd/ticket/2059
> >
> >If len % SSSSRV_PACKET_MEM_SIZE == 0 or some low number,
> >we can end up with totlen < len and return EINVAL.
> >
> >It also does not pad the length, but usually allocates
> >much more memory than is desired.
> >
> >len = 1024
> >n = 1024 % 512 + 1 = 0 + 1 = 1
> >totlen = 1 * 512 = 512
> >=> totlen < len
> >
> >len = 511
> >n = 511 % 512 + 1 = 511 + 1
> >totlen = 512 * 512 = 262144
> >totlen is way bigger than it was supposed to be
> >---
> > src/responder/common/responder_packet.c | 2 +-
> > 1 file changed, 1 insertion(+), 1 deletion(-)
> >
> >diff --git a/src/responder/common/responder_packet.c
b/src/responder/common/responder_packet.c
> >index
5132d955b71eb025cf54a41edd41da779d352f2e..6476bd6e59d6845042ab97bbc1846f9c76f68b57 100644
> >--- a/src/responder/common/responder_packet.c
> >+++ b/src/responder/common/responder_packet.c
> >@@ -105,7 +105,7 @@ int sss_packet_grow(struct sss_packet *packet, size_t size)
> >
> > /* make sure we do not overflow */
> > if (totlen < len) {
> >- int n = len % SSSSRV_PACKET_MEM_SIZE + 1;
> >+ int n = len / SSSSRV_PACKET_MEM_SIZE + 1;
> > totlen += n * SSSSRV_PACKET_MEM_SIZE;
> > if (totlen < len) {
> > return EINVAL;
> len = 1024
> n = 1024 / 512 +1
> n = 3
> totlen = 3* 512
> totlen = 1536
>
> and sufficient length is 1024
>
> what about
>
> int n = (len + SSSSRV_PACKET_MEM_SIZE -1) / SSSSRV_PACKET_MEM_SIZE;
No we are not aiming at perfect match in case len is a perfect multiple
of SSSSRV_PACKET_MEM_SIZE, 1536 is a good answer as it overallocates the
buffer and gives us more space to work with.
Overallocating by a full chunk is ok and desirable as it allow the body
to grow without yet another allocation. If we wanted perfect buffer
lengths we wouldn't allocate in chunks.
Ack to Pavel's patch.
Simo.
Pushed to master, sssd-1-10 and sssd-1-9