[PATCH] s4-libcli: Fix comparsion of uninitalized bytes.
simo
idra at samba.org
Tue Jul 2 07:09:59 MDT 2013
On Tue, 2013-07-02 at 14:36 +0200, Volker Lendecke wrote:
> On Tue, Jul 02, 2013 at 12:23:48PM +0200, Andreas Schneider wrote:
> > Found by valgrind.
> >
> > Signed-off-by: Andreas Schneider <asn at samba.org>
> > ---
> > source4/libcli/clireadwrite.c | 2 ++
> > 1 file changed, 2 insertions(+)
> >
> > diff --git a/source4/libcli/clireadwrite.c b/source4/libcli/clireadwrite.c
> > index 7d8f34a..7d73e75 100644
> > --- a/source4/libcli/clireadwrite.c
> > +++ b/source4/libcli/clireadwrite.c
> > @@ -38,6 +38,8 @@ ssize_t smbcli_read(struct smbcli_tree *tree, int fnum, void *_buf, off_t offset
> > return 0;
> > }
> >
> > + memset(buf, 0, size);
> > +
> > parms.readx.level = RAW_READ_READX;
> > parms.readx.in.file.fnum = fnum;
>
> To be honest, I don't understand why this should be
> necessary. I thought the idea of the smbcli_read call would
> be to download something from the server and fill it into
> "_buf". This would make the memory initialized. Why does
> valgrind require to first initialize something that we
> overwrite later anyway?
>
> Don't get me wrong. I am not against fixing a valgrind bug.
> I just want to understand this.
I would guess valgrind may be complaining if thwere are some cases where
we allocate the buffer of size S, then read N < S bytes and when
returned the caller for some reason copies the full buffer or access any
bytes > S but < N. In that case the client would be accessing
uninitialized memory.
In test_session_reauth1() for example we compare buf to data, without
length checks, so if the code returned less then dlen bytes you'd get
valgrind errors about reading uninitialized data from the stack.
I think that we should check the size a not initialize buf
unconditionally to suppress valgrind warnings. It's a bug in the code
that we access data that is not initialized, so we should fix the source
of the issue instead.
Simo.
More information about the samba-technical
mailing list