Using struct concat_buf and its associated concat_printf() variant
of snprintf(), replacing shprintf() macro.
This spares explicit checkguards against buffer overflow.
Signed-off-by: Alexander Nezhinsky <alexandern@xxxxxxxxxxxx>
---
usr/tgtadm.c | 83 ++++++++++++++++++++++++++++++++++-----------------------
1 files changed, 49 insertions(+), 34 deletions(-)
diff --git a/usr/tgtadm.c b/usr/tgtadm.c
index 41e9e99..f6b8342 100644
--- a/usr/tgtadm.c
+++ b/usr/tgtadm.c
@@ -415,6 +415,32 @@ static int str_to_op(char *str)
}
}
+int concat_printf(struct concat_buf *b, const char *format, ...)
+{
+ va_list args;
+ int rest = b->size - b->used;
+ int nprinted;
+
+ if (b->err)
+ return b->err;
+ if (!rest)
+ return (b->err = -ENOSPC);
+
+ va_start(args, format);
+ nprinted = vsnprintf(b->buf + b->used, rest, format, args);
+ va_end(args);
+
+ if (nprinted < 0)
+ return (b->err = nprinted);
+
+ if (nprinted >= rest) {
+ nprinted = rest;
+ b->err = -ENOSPC;
+ }
+ b->used += nprinted;
+ return b->err;
+}
+
static void bad_optarg(int ret, int ch, char *optarg)
{
if (ret == ERANGE)
@@ -445,26 +471,26 @@ static int verify_mode_params(int argc, char **argv, char *allowed)
int main(int argc, char **argv)
{
int ch, longindex, rc;
- int op, total, tid, rest, mode, dev_type, ac_dir;
+ int op, tid, mode, dev_type, ac_dir;
uint32_t cid, hostno;
uint64_t sid, lun, force;
- char *name, *value, *path, *targetname, *params, *address, *iqnname, *targetOps;
+ char *name, *value, *path, *targetname, *address, *iqnname, *targetOps;
char *portalOps, *bstype;
char *bsoflags;
char *blocksize;
char *user, *password;
char *buf;
size_t bufsz = BUFSIZE + sizeof(struct tgtadm_req);
+ struct concat_buf b;
struct tgtadm_req *req;
op = tid = mode = -1;
- total = cid = hostno = sid = 0;
+ cid = hostno = sid = 0;
lun = UINT64_MAX;
rc = 0;
dev_type = TYPE_DISK;
ac_dir = ACCOUNT_TYPE_INCOMING;
- rest = BUFSIZE;
name = value = path = targetname = address = iqnname = NULL;
targetOps = portalOps = bstype = NULL;
bsoflags = blocksize = user = password = NULL;
@@ -864,53 +890,42 @@ int main(int argc, char **argv)
req->ac_dir = ac_dir;
req->force = force;
- params = buf + sizeof(*req);
+ concat_buf_init(&b, buf + sizeof(*req), bufsz - sizeof(*req));
if (name)
- shprintf(total, params, rest, "%s=%s", name, value);
+ concat_printf(&b, "%s=%s", name, value);
if (path)
- shprintf(total, params, rest, "%spath=%s",
- rest == BUFSIZE ? "" : ",", path);
+ concat_printf(&b, "%spath=%s", concat_delim(&b,","), path);
if (req->device_type == TYPE_TAPE)
- shprintf(total, params, rest, "%sbstype=%s",
- rest == BUFSIZE ? "" : ",", "ssc");
+ concat_printf(&b, "%sbstype=%s", concat_delim(&b,","), "ssc");
else if (bstype)
- shprintf(total, params, rest, "%sbstype=%s",
- rest == BUFSIZE ? "" : ",", bstype);
+ concat_printf(&b, "%sbstype=%s", concat_delim(&b,","), bstype);
if (bsoflags)
- shprintf(total, params, rest, "%sbsoflags=%s",
- rest == BUFSIZE ? "" : ",", bsoflags);
+ concat_printf(&b, "%sbsoflags=%s", concat_delim(&b,","), bsoflags);
if (blocksize)
- shprintf(total, params, rest, "%sblocksize=%s",
- rest == BUFSIZE ? "" : ",", blocksize);
+ concat_printf(&b, "%sblocksize=%s", concat_delim(&b,","), blocksize);
if (targetname)
- shprintf(total, params, rest, "%stargetname=%s",
- rest == BUFSIZE ? "" : ",", targetname);
+ concat_printf(&b, "%stargetname=%s", concat_delim(&b,","), targetname);
if (address)
- shprintf(total, params, rest, "%sinitiator-address=%s",
- rest == BUFSIZE ? "" : ",", address);
+ concat_printf(&b, "%sinitiator-address=%s", concat_delim(&b,","), address);
if (iqnname)
- shprintf(total, params, rest, "%sinitiator-name=%s",
- rest == BUFSIZE ? "" : ",", iqnname);
+ concat_printf(&b, "%sinitiator-name=%s", concat_delim(&b,","), iqnname);
if (user)
- shprintf(total, params, rest, "%suser=%s",
- rest == BUFSIZE ? "" : ",", user);
+ concat_printf(&b, "%suser=%s", concat_delim(&b,","), user);
if (password)
- shprintf(total, params, rest, "%spassword=%s",
- rest == BUFSIZE ? "" : ",", password);
+ concat_printf(&b, "%spassword=%s", concat_delim(&b,","), password);
/* Trailing ',' makes parsing params in modules easier.. */
if (targetOps)
- shprintf(total, params, rest, "%stargetOps %s,",
- rest == BUFSIZE ? "" : ",", targetOps);
+ concat_printf(&b, "%stargetOps %s,", concat_delim(&b,","), targetOps);
if (portalOps)
- shprintf(total, params, rest, "%sportalOps %s,",
- rest == BUFSIZE ? "" : ",", portalOps);
+ concat_printf(&b, "%sportalOps %s,", concat_delim(&b,","), portalOps);
- req->len = sizeof(*req) + total;
+ if (b.err) {
+ eprintf("BUFSIZE (%d bytes) isn't long enough\n", BUFSIZE);
+ return EINVAL;
+ }
+ req->len = sizeof(*req) + b.used;
return ipc_mgmt_req(req);
-overflow:
- eprintf("BUFSIZE (%d bytes) isn't long enough\n", BUFSIZE);
- return EINVAL;
}
--
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
