[PATCH 4/4] using concat_printf() in iscsi code

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Using struct concat_buf and its associated concat_printf() variant
of snprintf(), replacing buffer_check() and _buffer_check() macros.
This spares explicit checkguards against buffer overflow, and by
explicit passing of concat_buf as the function argument allows
nested calls to functions which append more lines to the buffer.

Signed-off-by: Alexander Nezhinsky <alexandern@xxxxxxxxxxxx>
---
 usr/iscsi/iscsid.h |   12 +----
 usr/iscsi/isns.c   |   22 ++++-----
 usr/iscsi/target.c |  127 ++++++++++++++++++---------------------------------
 3 files changed, 56 insertions(+), 105 deletions(-)

diff --git a/usr/iscsi/iscsid.h b/usr/iscsi/iscsid.h
index c17ed13..e8d625b 100644
--- a/usr/iscsi/iscsid.h
+++ b/usr/iscsi/iscsid.h
@@ -28,6 +28,7 @@
 #include "param.h"
 #include "log.h"
 #include "tgtd.h"
+#include "util.h"
 
 #include "iscsi_proto.h"
 #include "iscsi_if.h"
@@ -351,19 +352,10 @@ extern void iscsi_exit(void);
 /* isns.c */
 extern int isns_init(void);
 extern void isns_exit(void);
-extern int isns_show(char *buf, int rest);
+extern int isns_show(struct concat_buf *b);
 extern int isns_update(char *params);
 extern int isns_scn_access(int tid, char *name);
 extern int isns_target_register(char *name);
 extern int isns_target_deregister(char *name);
 
-#define buffer_check(buf, total, len, rest)	\
-({						\
-	buf += len;				\
-	total += len;				\
-	rest -= len;				\
-	if (!rest)				\
-		break;				\
-})
-
 #endif	/* ISCSID_H */
diff --git a/usr/iscsi/isns.c b/usr/iscsi/isns.c
index 0afd251..fbe73ac 100644
--- a/usr/iscsi/isns.c
+++ b/usr/iscsi/isns.c
@@ -1068,20 +1068,16 @@ void isns_exit(void)
 	free(rxbuf);
 }
 
-int isns_show(char *buf, int rest)
+int isns_show(struct concat_buf *b)
 {
-	int total = 0, max = rest;
-
-	shprintf(total, buf, rest, "iSNS:\n");
-	shprintf(total, buf, rest, _TAB1 "iSNS=%s\n",
-		 use_isns ? "On" : "Off");
-	shprintf(total, buf, rest, _TAB1 "iSNSServerIP=%s\n", isns_addr);
-	shprintf(total, buf, rest, _TAB1 "iSNSServerPort=%d\n", isns_port);
-	shprintf(total, buf, rest, _TAB1 "iSNSAccessControl=%s\n",
-		 use_isns_ac ? "On" : "Off");
-	return total;
-overflow:
-	return max;
+	concat_printf(b, "iSNS:\n");
+	concat_printf(b, _TAB1 "iSNS=%s\n", use_isns ? "On" : "Off");
+	concat_printf(b, _TAB1 "iSNSServerIP=%s\n", isns_addr);
+	concat_printf(b, _TAB1 "iSNSServerPort=%d\n", isns_port);
+	concat_printf(b, _TAB1 "iSNSAccessControl=%s\n",
+		      use_isns_ac ? "On" : "Off");
+
+	return concat_buf_ret(b);
 }
 
 enum {
diff --git a/usr/iscsi/target.c b/usr/iscsi/target.c
index 9546035..ab8f11b 100644
--- a/usr/iscsi/target.c
+++ b/usr/iscsi/target.c
@@ -36,6 +36,7 @@
 #include "tgtadm.h"
 #include "tgtd.h"
 #include "target.h"
+#include "util.h"
 
 LIST_HEAD(iscsi_targets_list);
 
@@ -562,30 +563,20 @@ int iscsi_target_update(int mode, int op, int tid, uint64_t sid, uint64_t lun,
 	return err;
 }
 
-static int show_iscsi_param(char *buf, struct param *param, int rest)
+static int show_iscsi_param(struct param *param, struct concat_buf *b)
 {
-	int i, len, total;
-	char value[64];
 	struct iscsi_key *keys = session_keys;
+	int i;
+	char value[64];
 
-	for (i = total = 0; session_keys[i].name; i++) {
+	for (i = 0; session_keys[i].name; i++) {
 		param_val_to_str(keys, i, param[i].val, value);
-		len = snprintf(buf, rest, "%s=%s\n", keys[i].name, value);
-		buffer_check(buf, total, len, rest);
+		concat_printf(b, "%s=%s\n", keys[i].name, value);
 	}
 
-	return total;
+	return concat_buf_ret(b);
 }
 
-#define __buffer_check(buf, total, len, rest)	\
-({						\
-	buf += len;				\
-	total += len;				\
-	rest -= len;				\
-	if (!rest)				\
-		return total;			\
-})
-
 static struct iscsi_session *iscsi_target_find_session(
 	struct iscsi_target *target,
 	uint64_t sid)
@@ -602,26 +593,21 @@ static struct iscsi_session *iscsi_target_find_session(
 }
 
 static int iscsi_target_show_session(struct iscsi_target *target, uint64_t sid,
-				     char *buf, int rest)
+				     struct concat_buf *b)
 {
-	int len = 0, total = 0;
 	struct iscsi_session *session;
 
 	session = iscsi_target_find_session(target, sid);
 
-	if (session) {
-		len = show_iscsi_param(buf, session->session_param, rest);
-		__buffer_check(buf, total, len, rest);
-
-	}
+	if (session)
+		show_iscsi_param(session->session_param, b);
 
-	return total;
+	return concat_buf_ret(b);
 }
 
 static int iscsi_target_show_stats(struct iscsi_target *target, uint64_t sid,
-				   char *buf, int rest)
+				   struct concat_buf *b)
 {
-	int len = 0, total = 0;
 	struct iscsi_session *session;
 	struct iscsi_connection *conn;
 
@@ -629,8 +615,7 @@ static int iscsi_target_show_stats(struct iscsi_target *target, uint64_t sid,
 
 	if (session) {
 		list_for_each_entry(conn, &session->conn_list, clist) {
-			len = snprintf(buf, rest,
-				       "rxdata_octets: %" PRIu64 "\n"
+			concat_printf(b, "rxdata_octets: %" PRIu64 "\n"
 				       "txdata_octets: %" PRIu64 "\n"
 				       "dataout_pdus:  %d\n"
 				       "datain_pdus:   %d\n"
@@ -642,30 +627,27 @@ static int iscsi_target_show_stats(struct iscsi_target *target, uint64_t sid,
 				       conn->stats.datain_pdus,
 				       conn->stats.scsicmd_pdus,
 				       conn->stats.scsirsp_pdus);
-			__buffer_check(buf, total, len, rest);
 		}
 	}
 
-	return total;
+	return concat_buf_ret(b);
 
 }
 
 static int iscsi_target_show_connections(struct iscsi_target *target,
 					 uint64_t sid,
-					 char *buf, int rest)
+					 struct concat_buf *b)
 {
 	struct iscsi_session *session;
 	struct iscsi_connection *conn;
 	char addr[128];
-	int len = 0, total = 0;
 
 	list_for_each_entry(session, &target->sessions_list, slist) {
 		list_for_each_entry(conn, &session->conn_list, clist) {
 			memset(addr, 0, sizeof(addr));
 			conn->tp->ep_show(conn, addr, sizeof(addr));
 
-
-			len = snprintf(buf, rest, "Session: %u\n"
+			concat_printf(b, "Session: %u\n"
 				_TAB1 "Connection: %u\n"
 				_TAB2 "Initiator: %s\n"
 				_TAB2 "%s\n",
@@ -673,16 +655,14 @@ static int iscsi_target_show_connections(struct iscsi_target *target,
 				conn->cid,
 				session->initiator,
 				addr);
-			__buffer_check(buf, total, len, rest);
 		}
 	}
-	return total;
+	return concat_buf_ret(b);
 }
 
 static int iscsi_target_show_portals(struct iscsi_target *target, uint64_t sid,
-				   char *buf, int rest)
+				     struct concat_buf *b)
 {
-	int len = 0, total = 0;
 	struct iscsi_portal *portal;
 
 	list_for_each_entry(portal, &iscsi_portals_list,
@@ -690,57 +670,43 @@ static int iscsi_target_show_portals(struct iscsi_target *target, uint64_t sid,
 		int is_ipv6;
 
 		is_ipv6 = strchr(portal->addr, ':') != NULL;
-		len = snprintf(buf, rest,
-			       "Portal: %s%s%s:%d,%d\n",
+		concat_printf(b, "Portal: %s%s%s:%d,%d\n",
 			       is_ipv6 ? "[" : "",
 			       portal->addr,
 			       is_ipv6 ? "]" : "",
 			       portal->port ? portal->port : ISCSI_LISTEN_PORT,
 			       portal->tpgt);
-		__buffer_check(buf, total, len, rest);
 	}
 
-	return total;
-
+	return concat_buf_ret(b);
 }
 
-static int show_redirect_info(struct iscsi_target* target, char *buf, int rest)
+static int show_redirect_info(struct iscsi_target* target, struct concat_buf *b)
 {
-	int len, total = 0;
-
-	len = snprintf(buf, rest, "RedirectAddress=%s\n", target->redirect_info.addr);
-	__buffer_check(buf, total, len, rest);
-	len = snprintf(buf, rest, "RedirectPort=%s\n", target->redirect_info.port);
-	__buffer_check(buf, total, len, rest);
-	if (target->redirect_info.reason == ISCSI_LOGIN_STATUS_TGT_MOVED_TEMP) {
-		len = snprintf(buf, rest, "RedirectReason=Temporary\n");
-		__buffer_check(buf, total, len, rest);
-	} else if (target->redirect_info.reason == ISCSI_LOGIN_STATUS_TGT_MOVED_PERM) {
-		len = snprintf(buf, rest, "RedirectReason=Permanent\n");
-		__buffer_check(buf, total, len, rest);
-	} else {
-		len = snprintf(buf, rest, "RedirectReason=Unknown\n");
-		__buffer_check(buf, total, len, rest);
-	}
+	concat_printf(b, "RedirectAddress=%s\n", target->redirect_info.addr);
+	concat_printf(b, "RedirectPort=%s\n", target->redirect_info.port);
+	if (target->redirect_info.reason == ISCSI_LOGIN_STATUS_TGT_MOVED_TEMP)
+		concat_printf(b, "RedirectReason=Temporary\n");
+	else if (target->redirect_info.reason == ISCSI_LOGIN_STATUS_TGT_MOVED_PERM)
+		concat_printf(b, "RedirectReason=Permanent\n");
+	else
+		concat_printf(b, "RedirectReason=Unknown\n");
 
-	return total;
+	return concat_buf_ret(b);
 }
 
-static int show_redirect_callback(struct iscsi_target *target, char *buf, int rest)
+static int show_redirect_callback(struct iscsi_target *target, struct concat_buf *b)
 {
-	int len, total = 0;
-
-	len = snprintf(buf, rest, "RedirectCallback=%s\n", target->redirect_info.callback);
-	__buffer_check(buf, total, len, rest);
+	concat_printf(b, "RedirectCallback=%s\n", target->redirect_info.callback);
 
-	return total;
+	return concat_buf_ret(b);
 }
 
 int iscsi_target_show(int mode, int tid, uint64_t sid, uint32_t cid, uint64_t lun,
 		      char *buf, int rest)
 {
 	struct iscsi_target* target = NULL;
-	int len, total = 0;
+	struct concat_buf b;
 
 	if (mode != MODE_SYSTEM && mode != MODE_PORTAL) {
 	    target = target_find_by_id(tid);
@@ -748,38 +714,35 @@ int iscsi_target_show(int mode, int tid, uint64_t sid, uint32_t cid, uint64_t lu
 		    return -TGTADM_NO_TARGET;
 	}
 
+	concat_buf_init(&b, buf, rest);
+
 	switch (mode) {
 	case MODE_SYSTEM:
-		total = isns_show(buf, rest);
+		isns_show(&b);
 		break;
 	case MODE_TARGET:
 		if (target->redirect_info.callback)
-			len = show_redirect_callback(target, buf, rest);
+			show_redirect_callback(target, &b);
 		else if (strlen(target->redirect_info.addr))
-			len = show_redirect_info(target, buf, rest);
+			show_redirect_info(target, &b);
 		else
-			len = show_iscsi_param(buf, target->session_param, rest);
-		total += len;
+			show_iscsi_param(target->session_param, &b);
 		break;
 	case MODE_SESSION:
-		len = iscsi_target_show_session(target, sid, buf, rest);
-		total += len;
+		iscsi_target_show_session(target, sid, &b);
 		break;
 	case MODE_PORTAL:
-		len = iscsi_target_show_portals(target, sid, buf, rest);
-		total += len;
+		iscsi_target_show_portals(target, sid, &b);
 		break;
 	case MODE_CONNECTION:
-		len = iscsi_target_show_connections(target, sid, buf, rest);
-		total += len;
+		iscsi_target_show_connections(target, sid, &b);
 		break;
 	case MODE_STATS:
-		len = iscsi_target_show_stats(target, sid, buf, rest);
-		total += len;
+		iscsi_target_show_stats(target, sid, &b);
 		break;
 	default:
 		break;
 	}
 
-	return total;
+	return concat_buf_ret(&b);
 }
--
1.7.1
--
To unsubscribe from this list: send the line "unsubscribe stgt" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[Index of Archives]     [Linux SCSI]     [Linux RAID]     [Linux Clusters]     [Linux USB Devel]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]

  Powered by Linux