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