Opened 10 years ago

Closed 10 years ago

Last modified 10 years ago

#29 closed defect (fixed)

FreeBSD: cam_close_device() called twice

Reported by: Dan Lukes Owned by: somebody
Priority: major Milestone: Release 5.39.1
Component: smartd Version: 5.39
Keywords: freebsd cam double free corruption memory Cc: samm@…, Dan Lukes

Description

Function:
os_freebsd.cpp/freebsd_smart_interface::autodetect_smart_device()

line 1800

Relevant part of the code:

1796	        // now check if we are working with USB device, see umass.c
1797	        if(strcmp(ccb.cpi.dev_name,"umass-sim") == 0) { // USB device found
1798	          usbdevlist(bus,vendor_id, product_id, version);
1799	          int bus=ccb.cpi.unit_number; // unit_number will match umass number
1800	          cam_close_device(cam_dev);
1801	          if(usbdevlist(bus,vendor_id, product_id, version)){
1802	            const char * usbtype = get_usb_dev_type_by_id(vendor_id, product_id, version);
1803	            if (!usbtype)
1804	              return false;
1805	            return get_sat_device(usbtype, new freebsd_scsi_device(this, name, ""));
1806	          }
1807	        }
1808	#if FREEBSDVER > 800100
1809	        // check if we have ATA device connected to CAM (ada)
1810	        if(ccb.cpi.protocol == PROTO_ATA){
1811	          cam_close_device(cam_dev);
1812	          return new freebsd_atacam_device(this, name, "");
1813	        }
1814	#endif
1815	        // close cam device, we don`t need it anymore
1816	        cam_close_device(cam_dev);

The first cam_close_device(cam_dev) is called within if(strcmp(ccb.cpi.dev_name,"umass-sim") == 0) block. But such block may not end with return

In such case the second cam_close_device(cam_dev) will be called on line 1811 or 1816.

Double-close may cause several problems including memory corruption.

It seems to be a simple typo. It's Alex's code (AFAIK), so I hope he can correct it easily. If he will not repair it then I will assign this ticket later to myself.


Credits to Axel Beckert and Petr Salinger for it's help with problem identification and partial analysis.
See also http://bugs.debian.org/cgi-bin/bugreport.cgi?bug=561113

Change History (3)

comment:1 Changed 10 years ago by Dan Lukes

Cc: Dan Lukes added

comment:2 Changed 10 years ago by Alex Samorukov

Resolution: fixed
Status: newclosed

Yes, my fault. patch committed

comment:3 Changed 10 years ago by Christian Franke

Milestone: Release 5.40Release 5.39.1
Note: See TracTickets for help on using tickets.