Fix possible cdb printing overflow

Reported by Chris Coulson (chrisccoulson) at:
#613

tcmu_cdb_print_info has a fixed size 193 bytes scratch buffer on the
stack,
defined here:

void tcmu_cdb_print_info(struct tcmu_device *dev,
                         const struct tcmulib_cmd *cmd,
                         const char *info)
{
        int i, n, bytes;
        char fix[CDB_FIX_SIZE], *buf;

        buf = fix;
Further down, it uses sprintf to write some bytes in 2-digit hex format
to
buf, with each byte occupying 3 bytes in buf:

        for (i = 0, n = 0; i < bytes; i++) {
                n += sprintf(buf + n, "%x ", cmd->cdb[i]);
        }
To avoid overflowing the stack buffer, there is a check earlier in the
function which performs a heap allocation if bytes is too large:

        if (bytes > CDB_FIX_SIZE) {
                buf = malloc(CDB_TO_BUF_SIZE(bytes));
                if (!buf) {
                        tcmu_dev_err(dev, "out of memory\n");
                        return;
                }
        }
But this check looks wrong. If bytes > 64, then the previously mentioned
for
loop will overflow the 193 byte stack buffer. I think it should be:

        if (bytes > CDB_FIX_BYTES) {
Correcting this isn't sufficient on its own though, as this check
doesn't
take in to account that an additional string (info) is written to buf
and
can also result in the stack buffer overflowing:

        if (info)
                n += sprintf(buf + n, "%s", info);

Signed-off-by: Mike Christie <mchristi@redhat.com>

api.c

@@ -314,14 +314,14 @@ size_t tcmu_memcpy_from_iovec(
 	return copied;
 }
 
-#define CDB_TO_BUF_SIZE(bytes) ((bytes) * 3 + 1)
+#define CDB_TO_BUF_SIZE(bytes) ((bytes) * 3 + 2)
 #define CDB_FIX_BYTES 64 /* 64 bytes for default */
 #define CDB_FIX_SIZE CDB_TO_BUF_SIZE(CDB_FIX_BYTES)
 void tcmu_cdb_print_info(struct tcmu_device *dev,
 			 const struct tcmulib_cmd *cmd,
 			 const char *info)
 {
-	int i, n, bytes;
+	int i, n, bytes, info_len = 0;
 	char fix[CDB_FIX_SIZE], *buf;
 
 	buf = fix;
@@ -330,8 +330,11 @@ void tcmu_cdb_print_info(struct tcmu_device *dev,
 	if (bytes < 0)
 		return;
 
-	if (bytes > CDB_FIX_SIZE) {
-		buf = malloc(CDB_TO_BUF_SIZE(bytes));
+	if (info)
+		info_len = strlen(info);
+
+	if (CDB_TO_BUF_SIZE(bytes) + info_len > CDB_FIX_SIZE) {
+		buf = malloc(CDB_TO_BUF_SIZE(bytes) + info_len);
 		if (!buf) {
 			tcmu_dev_err(dev, "out of memory\n");
 			return;
@@ -353,7 +356,7 @@ void tcmu_cdb_print_info(struct tcmu_device *dev,
 		tcmu_dev_dbg_scsi_cmd(dev, "%s", buf);
 	}
 
-	if (bytes > CDB_FIX_SIZE)
+	if (buf != fix)
 		free(buf);
 }