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

Joel Granados jgranado at redhat.com
Wed Nov 26 10:22:05 UTC 2008


Can you please send a `git diff`?

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

> Hi list!
> 
> 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.
> 
> There is at least one place where ped_exception_throw() is called
> with
> NULL : libparted/labels/dasd.c:243    [*]
> 
> I suggest to:
> a) PED_ASSERT(message != NULL) somewhere in ped_exception_throw()
> b) change the condition on line 22 to be just 'if (result < size)',
> because when vsnprintf() once returned -1, it will probably keep
> returning the same even with larger buffer.
> c) fix [*] to something more appropriate, such as:
> 
> ped_exception_throw(PED_EXCEPTION_ERROR, PED_EXCEPTION_OK,
>                    _("%s is corrupted"),
>                    dev->path);
> 
> Of course the 'is corrupted' text has to be changed to something
> better.
> 
> BTW isn't the while loop a typical case where realloc() would be more
> appropriate then malloc()/free() ?
> 
> 
> What do you guys think?
> 
> 
> -- 
> 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



More information about the parted-devel mailing list