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

[Petr Uzel]

Hi Joel and others!

On Mon, Feb 09, 2009 at 01:23:25PM -0500, Joel Granados wrote:
> 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?

No, you are not, but obviously I am.

Let me start from scratch and let's forget what I wrote before as
there were too many mistakes :)

About three moths ago, I was debugging parted on s390 - it went into
endless loop allocating memory. I'm pretty sure that this happened
inside the ped_exception_throw() function, which was called with weird
arguments (from mentioned dasd.c:243). Removing this call (as it does
not provide any useful information anyway) fixed it.

But obviously I was wrong with the ped_exception_throw() analysis - as
you correctly noted:
1) ped_exception_throw() is called with message==NULL==0 in dasd.c:243
2) in ped_exception_throw(), if message==NULL, it shouldn't go into
the while loop at all.

But I'm pretty sure it went into that loop. The problem is that I have
no longer access to that s390 machine where this was easily
reproduced. Looking into the ped_exception_throw() code it is not
clear to me what actually happened there: the only thing that comes to
my mind now is that the function can be called with variadic
arguments, but these are missing - can this make any troubles on s390
(IMHO it is OK on more common architectures) ?

I'm really confused now.

Thanks to Joel for correcting me and apologies for the noise here.


-- 
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