[parted-devel] BUG: ped_exception_throw() can go to endless loop allocating memory

[Joel Granados]

I think you are mistaken.  This is the output of masters dasd.c file (not all of it, just the relevant lines.)

213 static int
214 dasd_probe (const PedDevice *dev)
215 {
216 |___char *errstr = 0;
217 |___LinuxSpecific* arch_specific;
218 |___struct fdasd_anchor anchor;
219 
220 |___PED_ASSERT(dev != NULL, return 0);
221 
222 |___if (!(dev->type == PED_DEVICE_DASD || dev->type == PED_DEVICE_VIODASD))
223 |___|___return 0;
224 
225 |___arch_specific = LINUX_SPECIFIC(dev);
226 
227 |___/* add partition test here */
228 |___fdasd_initialize_anchor(&anchor);
229 
230 |___fdasd_get_geometry(&anchor, arch_specific->fd);
231 
232 |___fdasd_check_api_version(&anchor, arch_specific->fd);
233 
234 |___if (fdasd_check_volume(&anchor, arch_specific->fd))
235 |___|___goto error_cleanup;
236 
237 |___fdasd_cleanup(&anchor);
238 
239 |___return 1;
240 
241 error_cleanup:
242 |___fdasd_cleanup(&anchor);
243 |___ped_exception_throw(PED_EXCEPTION_ERROR,PED_EXCEPTION_IGNORE_CANCEL,errstr);
244 
245 |___return 0;
246 }


Now.  On line 216, errstr is initialized to 0, which when evaluated in the while will do the same thing as the NULL.  Additionally, I don't see any other place where it is modified in the function before getting to line 243, where it is called with the 0 value.

Am I missing something?

regards
----- "Petr Uzel" <petr.uzel at suse.cz> wrote:

> Hi and thanks for response!
> 
> On Mon, Feb 09, 2009 at 12:47:37PM -0500, Joel Granados wrote:
> > > 1	PedExceptionOption
> > > 2	ped_exception_throw (PedExceptionType ex_type,
> > > 3			     PedExceptionOption ex_opts, const char* message, ...)
> > > 4	{
> > > 5		va_list		arg_list;
> > > 6		int result;
> > > 7		static int size = 1000;
> > > 
> > > 8		if (ex)
> > > 9			ped_exception_catch ();
> > > 
> > > 10		ex = (PedException*) malloc (sizeof (PedException));
> > > 11		if (!ex)
> > > 12			goto no_memory;
> > > 
> > > 13		ex->type = ex_type;
> > > 14		ex->options = ex_opts;
> > > 
> > > 15		while (message) {
> > > 16				ex->message = (char*) malloc (size * sizeof (char));
> > > 17				if (!ex->message)
> > > 18						goto no_memory;
> > > 
> > > 19				va_start (arg_list, message);
> > > 20				result = vsnprintf (ex->message, size, message, arg_list);
> > > 21				va_end (arg_list);
> > > 
> > > 22				if (result > -1 && result < size)
> > > 23						break;
> > > 
> > > 24				size += 10;
> > > 25				free (ex->message);
> > > 26		}
> > > 
> > > 27		return do_throw ();
> > > 
> > > If this function gets NULL in 'message' parameter, it will go
> into
> > > endless loop allocating memory because vsnprintf() on line 20
> will
> > > keep returning -1 and thus the condition on line 22 will never be
> > > true.
> 
> In the above analysis, I've mixed up two different things: NULL and
> ""
> (empty zero-terminated string). I wrote above NULL case but had "" in
> mind - sorry for the mess.
> 
> > 
> > Actually this is inaccurate.  I will never reach vsnprintf.  If
> > message is NULL then the while will evaluate to false and continue
> > on line 27.  However, the fact that we are calling the exception
> > function without a message at all is a cause for worry.  This would
> > mean that the user could potentially be misinformed or worse, not
> > informed at all.
> 
> Correct: with message==NULL, it will behave this way. But with
> message="" (what I've meant before), it will go into endless loop
> allocating memory. ped_exception_throw() is called this way in
> libparted/labels/dasd.c:243
> 
> > 
> > I propose a messages like the following:
> > 
> > """
> > No correct volume label found in the device dev->name.
> > """
> > 
> > Suggestions for the message are appreciated.
> > 
> 
> 
> -- 
> Best regards / s pozdravem
> 
> Petr Uzel, Packages maintainer
> ---------------------------------------------------------------------
> SUSE LINUX, s.r.o.                          e-mail: puzel at suse.cz
> Lihovarská 1060/12                          tel: +420 284 028 964
> 190 00 Prague 9                             fax: +420 284 028 951
> Czech Republic                              http://www.suse.cz
> 
> _______________________________________________
> parted-devel mailing list
> parted-devel at lists.alioth.debian.org
> http://lists.alioth.debian.org/mailman/listinfo/parted-devel

-- 
Joel Andres Granados
Red Hat / Brno Czech Republic