[stgt] [PATCH 5/6] mgmt and concat_buf: concat_buf related changes throughout the code
Alexander Nezhinsky
alexandern at mellanox.com
Wed Jan 11 14:29:29 CET 2012
Use concat_buf in all functions producing output for management requests.
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 at mellanox.com>
---
usr/driver.c | 2 +-
usr/driver.h | 2 +-
usr/iscsi/iscsid.h | 14 +-----
usr/iscsi/isns.c | 22 ++++-----
usr/iscsi/target.c | 126 ++++++++++++++++++----------------------------------
usr/target.c | 94 ++++++++++++++++-----------------------
usr/tgtd.h | 8 ++-
7 files changed, 101 insertions(+), 167 deletions(-)
diff --git a/usr/driver.c b/usr/driver.c
index bf612b9..a564cf7 100644
--- a/usr/driver.c
+++ b/usr/driver.c
@@ -24,9 +24,9 @@
#include <inttypes.h>
#include "list.h"
+#include "util.h"
#include "tgtd.h"
#include "driver.h"
-#include "util.h"
#define MAX_NR_DRIVERS 32
diff --git a/usr/driver.h b/usr/driver.h
index 091c3b0..b7e454c 100644
--- a/usr/driver.h
+++ b/usr/driver.h
@@ -24,7 +24,7 @@ struct tgt_driver {
int (*lu_create)(struct scsi_lu *);
int (*update)(int, int, int ,uint64_t, uint64_t, uint32_t, char *);
- int (*show)(int, int, uint64_t, uint32_t, uint64_t, char *, int);
+ int (*show)(int, int, uint64_t, uint32_t, uint64_t, struct concat_buf *);
uint64_t (*scsi_get_lun)(uint8_t *);
diff --git a/usr/iscsi/iscsid.h b/usr/iscsi/iscsid.h
index c17ed13..4f37604 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"
@@ -335,7 +336,7 @@ extern int iqn_acl(int tid, struct iscsi_connection *conn);
extern int iscsi_target_create(struct target *);
extern void iscsi_target_destroy(int tid, int force);
extern int iscsi_target_show(int mode, int tid, uint64_t sid, uint32_t cid,
- uint64_t lun, char *buf, int rest);
+ uint64_t lun, struct concat_buf *b);
int iscsi_target_update(int mode, int op, int tid, uint64_t sid, uint64_t lun,
uint32_t cid, char *name);
int target_redirected(struct iscsi_target *target,
@@ -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..1f1852c 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 TGTADM_SUCCESS;
}
enum {
diff --git a/usr/iscsi/target.c b/usr/iscsi/target.c
index 9546035..eb3afac 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 TGTADM_SUCCESS;
}
-#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 TGTADM_SUCCESS;
}
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 TGTADM_SUCCESS;
}
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 TGTADM_SUCCESS;
}
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,42 @@ 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 TGTADM_SUCCESS;
}
-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 TGTADM_SUCCESS;
}
-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 TGTADM_SUCCESS;
}
int iscsi_target_show(int mode, int tid, uint64_t sid, uint32_t cid, uint64_t lun,
- char *buf, int rest)
+ struct concat_buf *b)
{
struct iscsi_target* target = NULL;
- int len, total = 0;
if (mode != MODE_SYSTEM && mode != MODE_PORTAL) {
target = target_find_by_id(tid);
@@ -750,36 +715,31 @@ int iscsi_target_show(int mode, int tid, uint64_t sid, uint32_t cid, uint64_t lu
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 TGTADM_SUCCESS;
}
diff --git a/usr/target.c b/usr/target.c
index dd4b358..5d7aab1 100644
--- a/usr/target.c
+++ b/usr/target.c
@@ -1758,9 +1758,8 @@ static char *print_type(int type)
return name;
}
-int tgt_target_show_all(char *buf, int rest)
+int tgt_target_show_all(struct concat_buf *b)
{
- int total = 0, max = rest;
char strflags[128];
struct target *target;
struct scsi_lu *lu;
@@ -1769,8 +1768,7 @@ int tgt_target_show_all(char *buf, int rest)
struct it_nexus *nexus;
list_for_each_entry(target, &target_list, target_siblings) {
- shprintf(total, buf, rest,
- "Target %d: %s\n"
+ concat_printf(b, "Target %d: %s\n"
_TAB1 "System information:\n"
_TAB2 "Driver: %s\n"
_TAB2 "State: %s\n",
@@ -1779,18 +1777,18 @@ int tgt_target_show_all(char *buf, int rest)
tgt_drivers[target->lid]->name,
target_state_name(target->target_state));
- shprintf(total, buf, rest, _TAB1 "I_T nexus information:\n");
+ concat_printf(b, _TAB1 "I_T nexus information:\n");
list_for_each_entry(nexus, &target->it_nexus_list, nexus_siblings) {
- shprintf(total, buf, rest, _TAB2 "I_T nexus: %" PRIu64 "\n",
- nexus->itn_id);
+ concat_printf(b, _TAB2 "I_T nexus: %" PRIu64 "\n",
+ nexus->itn_id);
if (nexus->info)
- shprintf(total, buf, rest, "%s", nexus->info);
+ concat_printf(b, "%s", nexus->info);
}
- shprintf(total, buf, rest, _TAB1 "LUN information:\n");
+ concat_printf(b, _TAB1 "LUN information:\n");
list_for_each_entry(lu, &target->device_list, device_siblings)
- shprintf(total, buf, rest,
+ concat_printf(b,
_TAB2 "LUN: %" PRIu64 "\n"
_TAB3 "Type: %s\n"
_TAB3 "SCSI ID: %s\n"
@@ -1821,32 +1819,28 @@ int tgt_target_show_all(char *buf, int rest)
!strcmp(tgt_drivers[target->lid]->name, "iser")) {
int i, aid;
- shprintf(total, buf, rest, _TAB1
- "Account information:\n");
+ concat_printf(b, _TAB1 "Account information:\n");
for (i = 0; i < target->account.nr_inaccount; i++) {
aid = target->account.in_aids[i];
- shprintf(total, buf, rest, _TAB2 "%s\n",
- __account_lookup_id(aid)->user);
+ concat_printf(b, _TAB2 "%s\n",
+ __account_lookup_id(aid)->user);
}
if (target->account.out_aid) {
aid = target->account.out_aid;
- shprintf(total, buf, rest,
- _TAB2 "%s (outgoing)\n",
+ concat_printf(b, _TAB2 "%s (outgoing)\n",
__account_lookup_id(aid)->user);
}
}
- shprintf(total, buf, rest, _TAB1 "ACL information:\n");
+ concat_printf(b, _TAB1 "ACL information:\n");
list_for_each_entry(acl, &target->acl_list, aclent_list)
- shprintf(total, buf, rest, _TAB2 "%s\n", acl->address);
+ concat_printf(b, _TAB2 "%s\n", acl->address);
list_for_each_entry(iqn_acl, &target->iqn_acl_list, iqn_aclent_list)
- shprintf(total, buf, rest, _TAB2 "%s\n", iqn_acl->name);
+ concat_printf(b, _TAB2 "%s\n", iqn_acl->name);
}
- return total;
-overflow:
- return max;
+ return TGTADM_SUCCESS;
}
char *tgt_targetname(int tid)
@@ -2048,20 +2042,17 @@ int tgt_portal_destroy(int lld, char *args)
return 0;
}
-int account_show(char *buf, int rest)
+int account_show(struct concat_buf *b)
{
- int total = 0, max = rest;
struct account_entry *ac;
if (!list_empty(&account_list))
- shprintf(total, buf, rest, "Account list:\n");
+ concat_printf(b, "Account list:\n");
list_for_each_entry(ac, &account_list, account_siblings)
- shprintf(total, buf, rest, _TAB1 "%s\n", ac->user);
+ concat_printf(b, _TAB1 "%s\n", ac->user);
- return total;
-overflow:
- return max;
+ return TGTADM_SUCCESS;
}
static struct {
@@ -2102,9 +2093,8 @@ int system_set_state(char *str)
return err;
}
-int system_show(int mode, char *buf, int rest)
+int system_show(int mode, struct concat_buf *b)
{
- int total = 0, max = rest;
struct backingstore_template *bst;
struct device_type_template *devt;
int i;
@@ -2114,51 +2104,45 @@ int system_show(int mode, char *buf, int rest)
if (mode != MODE_SYSTEM)
return 0;
- shprintf(total, buf, rest, "System:\n");
- shprintf(total, buf, rest, _TAB1 "State: %s\n",
- system_state_name(sys_state));
+ concat_printf(b, "System:\n");
+ concat_printf(b, _TAB1 "State: %s\n", system_state_name(sys_state));
+ concat_printf(b, _TAB1 "debug: %s\n", is_debug ? "on" : "off");
- shprintf(total, buf, rest, "LLDs:\n");
+ concat_printf(b, "LLDs:\n");
for (i = 0; tgt_drivers[i]; i++) {
- shprintf(total, buf, rest, _TAB1 "%s: %s\n",
- tgt_drivers[i]->name,
- driver_state_name(tgt_drivers[i]));
+ concat_printf(b, _TAB1 "%s: %s\n", tgt_drivers[i]->name,
+ driver_state_name(tgt_drivers[i]));
}
- shprintf(total, buf, rest, "Backing stores:\n");
+ concat_printf(b, "Backing stores:\n");
list_for_each_entry(bst, &bst_list, backingstore_siblings) {
if (!bst->bs_oflags_supported)
- shprintf(total, buf, rest, _TAB1 "%s\n", bst->bs_name);
+ concat_printf(b, _TAB1 "%s\n", bst->bs_name);
else
- shprintf(total, buf, rest, _TAB1 "%s (bsoflags %s)\n",
- bst->bs_name,
- open_flags_to_str(strflags, bst->bs_oflags_supported));
+ concat_printf(b, _TAB1 "%s (bsoflags %s)\n", bst->bs_name,
+ open_flags_to_str(strflags, bst->bs_oflags_supported));
}
- shprintf(total, buf, rest, "Device types:\n");
+ concat_printf(b, "Device types:\n");
list_for_each_entry(devt, &device_type_list, device_type_siblings)
- shprintf(total, buf, rest, _TAB1 "%s\n", print_type(devt->type));
+ concat_printf(b, _TAB1 "%s\n", print_type(devt->type));
if (global_target.account.nr_inaccount) {
int i, aid;
- shprintf(total, buf, rest,
- "Account information:\n");
+ concat_printf(b, _TAB1 "%s\n", "Account information:\n");
for (i = 0; i < global_target.account.nr_inaccount; i++) {
aid = global_target.account.in_aids[i];
- shprintf(total, buf, rest, _TAB1 "%s\n",
- __account_lookup_id(aid)->user);
+ concat_printf(b, _TAB1 "%s\n",
+ __account_lookup_id(aid)->user);
}
if (global_target.account.out_aid) {
aid = global_target.account.out_aid;
- shprintf(total, buf, rest,
- _TAB1 "%s (outgoing)\n",
- __account_lookup_id(aid)->user);
+ concat_printf(b, _TAB1 "%s (outgoing)\n",
+ __account_lookup_id(aid)->user);
}
}
- return total;
-overflow:
- return max;
+ return TGTADM_SUCCESS;
}
int is_system_available(void)
diff --git a/usr/tgtd.h b/usr/tgtd.h
index 1805f3e..c60957e 100644
--- a/usr/tgtd.h
+++ b/usr/tgtd.h
@@ -4,6 +4,8 @@
#include "log.h"
#include "scsi_cmnd.h"
+struct concat_buf;
+
#define SCSI_ID_LEN 36
#define SCSI_SN_LEN 36
@@ -230,9 +232,9 @@ extern int tgt_device_path_update(struct target *target, struct scsi_lu *lu, cha
extern int tgt_target_create(int lld, int tid, char *args);
extern int tgt_target_destroy(int lld, int tid, int force);
extern char *tgt_targetname(int tid);
-extern int tgt_target_show_all(char *buf, int rest);
+extern int tgt_target_show_all(struct concat_buf *b);
int system_set_state(char *str);
-int system_show(int mode, char *buf, int rest);
+int system_show(int mode, struct concat_buf *b);
int is_system_available(void);
int is_system_inactive(void);
@@ -301,7 +303,7 @@ extern int account_lookup(int tid, int type, char *user, int ulen, char *passwor
extern int account_add(char *user, char *password);
extern int account_del(char *user);
extern int account_ctl(int tid, int type, char *user, int bind);
-extern int account_show(char *buf, int rest);
+extern int account_show(struct concat_buf *b);
extern int account_available(int tid, int dir);
extern int it_nexus_create(int tid, uint64_t itn_id, int host_no, char *info);
--
1.7.3.2
--
To unsubscribe from this list: send the line "unsubscribe stgt" in
the body of a message to majordomo at vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
More information about the stgt
mailing list