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

[Joel Granados]

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

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

mmm. interesting.  I'll try to test with what we have in the office.  In any case, if you encounter this again, it would be *great* to have a test case.


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

In any case, I think there is still something to rescue from this issue and it is the lack of an error message.  I will post a patch adding one.  hopefully today.

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