bin/57088: [PATCH] for a possible fd leak in libcam.c

[Rui Lopes]

>Number:         57088
>Category:       bin
>Synopsis:       [PATCH] for a possible fd leak in libcam.c
>Confidential:   no
>Severity:       non-critical
>Priority:       medium
>Responsible:    freebsd-bugs
>State:          open
>Quarter:        
>Keywords:       
>Date-Required:
>Class:          sw-bug
>Submitter-Id:   current-users
>Arrival-Date:   Mon Sep 22 07:20:11 PDT 2003
>Closed-Date:
>Last-Modified:
>Originator:     Rui Lopes
>Release:        FreeBSD 5.1-RELEASE-p5 i386
>Organization:
>Environment:
System: FreeBSD disty 5.1-RELEASE-p5 FreeBSD 5.1-RELEASE-p5 #2: Mon Sep 22 12:24:38 WEST 2003 root at disty:/usr/obj/usr/src/sys/DEBUGGER i386


	
>Description:
	
* Fixes one possible FD leak.
* Fixes some buffer overrun.
  NOTE: The others sprintf seem inoffensive, because we control what we place
        in the buffer, so I didn't change them.

>How-To-Repeat:
	
>Fix:

	

--- camlib.c.patch begins here ---
# * Fixes one possible FD leak.
# * Fixes some buffer overrun.
# -- Rui Lopes <rui at ruilopes.com>
--- camlib.c.orig	Mon Sep 22 14:24:52 2003
+++ camlib.c	Mon Sep 22 14:51:02 2003
@@ -388,8 +388,9 @@
 			   PERIPH_MATCH_LUN | PERIPH_MATCH_NAME;
 
 	if (ioctl(fd, CAMIOCOMMAND, &ccb) == -1) {
-		sprintf(cam_errbuf, "%s: CAMIOCOMMAND ioctl failed\n"
-			"%s: %s", func_name, func_name, strerror(errno));
+		snprintf(cam_errbuf, CAM_ERRBUF_SIZE,
+			 "%s: CAMIOCOMMAND ioctl failed\n%s: %s", func_name,
+			 func_name, strerror(errno));
 		goto btl_bailout;
 	}
 
@@ -515,6 +516,7 @@
 			 "%s: %s%s", func_name, func_name, strerror(errno),
 			 (errno == ENOENT) ? tmpstr : "");
 
+		close(fd);
 		return(NULL);
 	}
 
@@ -527,8 +529,9 @@
 	 * the device the user gave us.
 	 */
 	if (ccb.cgdl.status == CAM_GDEVLIST_ERROR) {
-		sprintf(cam_errbuf, "%s: device %s%d does not exist",
-			func_name, dev_name, unit);
+		snprintf(cam_errbuf, CAM_ERRBUF_SIZE,
+			 "%s: device %s%d does not exist", func_name, dev_name,
+			 unit);
 		return(NULL);
 	}
 
@@ -558,9 +561,9 @@
 	if (device == NULL) {
 		if ((device = (struct cam_device *)malloc(
 		     sizeof(struct cam_device))) == NULL) {
-			sprintf(cam_errbuf, "%s: device structure malloc"
-				" failed\n%s: %s", func_name, func_name,
-				strerror(errno));
+			snprintf(cam_errbuf, CAM_ERRBUF_SIZE,
+				 "%s: device structure malloc failed\n%s: %s",
+				 func_name, func_name, strerror(errno));
 			return(NULL);
 		}
 		device->fd = -1;
@@ -616,8 +619,9 @@
 		 * because we just opened it above.  The only way this
 		 * ioctl can fail is if the ccb size is wrong.
 		 */
-		sprintf(cam_errbuf, "%s: CAMGETPASSTHRU ioctl failed\n"
-			"%s: %s", func_name, func_name, strerror(errno));
+		snprintf(cam_errbuf, CAM_ERRBUF_SIZE,
+			 "%s: CAMGETPASSTHRU ioctl failed\n%s: %s", func_name,
+			 func_name, strerror(errno));
 		goto crod_bailout;
 	}
 
@@ -642,8 +646,9 @@
 
 	ccb.ccb_h.func_code = XPT_PATH_INQ;
 	if (ioctl(fd, CAMIOCOMMAND, &ccb) == -1) {
-		sprintf(cam_errbuf, "%s: Path Inquiry CCB failed\n"
-			"%s: %s", func_name, func_name, strerror(errno));
+		snprintf(cam_errbuf, CAM_ERRBUF_SIZE,
+			 "%s: Path Inquiry CCB failed\n%s: %s", func_name,
+			 func_name, strerror(errno));
 		goto crod_bailout;
 	}
 	strlcpy(device->sim_name, ccb.cpi.dev_name, sizeof(device->sim_name));
@@ -656,8 +661,9 @@
 	 */
 	ccb.ccb_h.func_code = XPT_GDEV_TYPE;
 	if (ioctl(fd, CAMIOCOMMAND, &ccb) == -1) {
-		sprintf(cam_errbuf, "%s: Get Device Type CCB failed\n"
-			"%s: %s", func_name, func_name, strerror(errno));
+		snprintf(cam_errbuf, CAM_ERRBUF_SIZE,
+			 "%s: Get Device Type CCB failed\n%s: %s", func_name,
+			 func_name, strerror(errno));
 		goto crod_bailout;
 	}
 	device->pd_type = SID_TYPE(&ccb.cgd.inq_data);
@@ -679,8 +685,9 @@
 	ccb.cts.flags = CCB_TRANS_CURRENT_SETTINGS;
 
 	if (ioctl(fd, CAMIOCOMMAND, &ccb) == -1) {
-		sprintf(cam_errbuf, "%s: Get Transfer Settings CCB failed\n"
-			"%s: %s", func_name, func_name, strerror(errno));
+		snprintf(cam_errbuf, CAM_ERRBUF_SIZE,
+			 "%s: Get Transfer Settings CCB failed\n%s: %s",
+			 func_name, func_name, strerror(errno));
 		goto crod_bailout;
 	}
 	device->sync_period = ccb.cts.sync_period;
--- camlib.c.patch ends here ---


>Release-Note:
>Audit-Trail:
>Unformatted: