[dm-devel] Segmentation Fault Question

[Wood, Brian J]

Hello everyone, I think I've found an issue in libdevmapper-event.c that
is generating a segmentation fault condition for me during some boundary
testing. I wanted to get some advice on how the patch I'm going to make
should fix this. Here's the snippet that leads to the segfault:

In the file libdevmapper-event.c at line 722 the _get_device_info() call
can return NULL to dmt on failure. This causes the leap to the "fail"
label where the null pointer is passed into dm_task_destroy(). 

   int dm_event_get_registered_device(struct dm_event_handler *dmevh,
int next)
   {
      .
      .
      .
      if (!(dmt = _get_device_info(dmevh))) {
		ret = -ENXIO; /* dmeventd probably gave us bogus uuid
back */
		goto fail;
	}
      .
      .
      .
      fail:
	if (msg.data)
		dm_free(msg.data);
	if (reply_dso)
		dm_free(reply_dso);
	if (reply_uuid)
		dm_free(reply_uuid);
	_dm_event_handler_clear_dev_info(dmevh);
	dm_task_destroy(dmt);


In the file libdm-iface.c at line 320 where dm_task_destroy() resides it
blindly uses the passed in pointer without checking if its NULL; this is
where the segmentation fault occurs.

   void dm_task_destroy(struct dm_task *dmt)
   {
	struct target *t, *n;

	for (t = dmt->head; t; t = n) {
      .
      .
      .


My question is should the patch insert a test condition of the pointer
before using it in the "for" loop (which is where I want to put the
fix)? Or is there another preferred way the maintainers of device-mapper
would like to handle error checking in this case?

 

Brian Wood
Intel Corporation 
Digital Enterprise Group
Manageability & Platform Software Division
brian.j.wood at intel.com