Re: [PATCH 1/1] s390x/s390-virtio-ccw: fix off-by-one in loadparm getter

From: Halil Pasic <pasic@linux.ibm.com>
To: "Daniel P. Berrangé" <berrange@redhat.com>
Cc: Peter Maydell <peter.maydell@linaro.org>,
	Thomas Huth <thuth@redhat.com>,
	David Hildenbrand <david@redhat.com>,
	Cornelia Huck <cohuck@redhat.com>,
	qemu-devel@nongnu.org,
	Christian Borntraeger <borntraeger@de.ibm.com>,
	qemu-s390x@nongnu.org, Richard Henderson <rth@twiddle.net>
Subject: Re: [PATCH 1/1] s390x/s390-virtio-ccw: fix off-by-one in loadparm getter
Date: Thu, 30 Jul 2020 13:28:12 +0200	[thread overview]
Message-ID: <20200730132812.0a4a4607.pasic@linux.ibm.com> (raw)
In-Reply-To: <20200730102506.GD3477223@redhat.com>

On Thu, 30 Jul 2020 11:25:06 +0100
Daniel P. Berrangé <berrange@redhat.com> wrote:

> >      /* make a NUL-terminated string */
> > -    loadparm_str = g_memdup(ms->loadparm, sizeof(ms->loadparm) + 1);
> > -    loadparm_str[sizeof(ms->loadparm)] = 0;
> > +    loadparm_str = g_malloc0(sizeof(ms->loadparm) + 1);
> > +    memcpy(loadparm_str, ms->loadparm, sizeof(ms->loadparm));  
> 
> I feel like  g_strndup(ms->loadparm, sizeof(ms->loadparm))
> is what should have been used here.
> 
> It copies N characters, but allocates N+1 adding a trailing NUL
> which are the semantic we wanted here.

I agree. Thanks for pointing this out. I'm not very familiar with the
string utilities of glib.

Regards,
Halil