Bug 1813491 - tcmu-runner: Fix possible cdb printing overflow
Summary: tcmu-runner: Fix possible cdb printing overflow
Keywords:
Status: CLOSED ERRATA
Alias: None
Product: Red Hat Gluster Storage
Classification: Red Hat Storage
Component: tcmu-runner
Version: ocs-3.11
Hardware: Unspecified
OS: Unspecified
unspecified
medium
Target Milestone: ---
: OCS 3.11.z Batch Update 6
Assignee: Xiubo Li
QA Contact: Vinayak Papnoi
URL:
Whiteboard:
Depends On:
Blocks:
TreeView+ depends on / blocked
 
Reported: 2020-03-14 02:09 UTC by Xiubo Li
Modified: 2020-12-17 04:31 UTC (History)
8 users (show)

Fixed In Version: tcmu-runner-1.2.0-33.el7rhgs
Doc Type: No Doc Update
Doc Text:
Clone Of:
Environment:
Last Closed: 2020-12-17 04:31:42 UTC
Embargoed:


Attachments (Terms of Use)


Links
System ID Private Priority Status Summary Last Updated
Red Hat Product Errata RHBA-2020:5602 0 None None None 2020-12-17 04:31:56 UTC

Description Xiubo Li 2020-03-14 02:09:57 UTC
Description of problem:
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);

Version-Release number of selected component (if applicable):


How reproducible:


Steps to Reproduce:
1.
2.
3.

Actual results:


Expected results:


Additional info:

Comment 11 errata-xmlrpc 2020-12-17 04:31:42 UTC
Since the problem described in this bug report should be
resolved in a recent advisory, it has been closed with a
resolution of ERRATA.

For information on the advisory (OpenShift Container Storage 3.11.z bug fix update), and where to find the updated
files, follow the link below.

If the solution does not work for you, open a new bug report.

https://access.redhat.com/errata/RHBA-2020:5602


Note You need to log in before you can comment on or make changes to this bug.