Project

General

Profile

Actions

Bug #13968

closed

tpm driver fails bounds checks

Added by Jason King 4 months ago. Updated 4 months ago.

Status:
Closed
Priority:
Normal
Assignee:
Category:
driver - device drivers
Start date:
Due date:
% Done:

100%

Estimated time:
Difficulty:
Medium
Tags:
Gerrit CR:

Description

Noticing that usr/src/uts/intel/tpm/Makefile currently silencing array bounds errors (as part of the initial smatch implementation), removing the silence gives the following errors:

/ws/illumos-gate/usr/src/tools/proto/root_i386-nd/opt/onbld/bin/i386/smatch: /ws/illumos-gate/usr/src/uts/common/io/tpm/tpm.c:691 tpm_get_ordinal_duration() error: buffer overflow 'tsc_ords_duration' 12 <= 12
/ws/illumos-gate/usr/src/tools/proto/root_i386-nd/opt/onbld/bin/i386/smatch: /ws/illumos-gate/usr/src/uts/common/io/tpm/tpm.c:701 tpm_get_ordinal_duration() error: buffer overflow 'tpm_ords_duration' 243 <= 243

Looking at the relevant code, we see:

        if (ordinal & TSC_ORDINAL_MASK) {
                if (ordinal > TSC_ORDINAL_MAX) {
#ifdef DEBUG
                        cmn_err(CE_WARN,
                            "!%s: tsc ordinal: %d exceeds MAX: %d",
                            myname, ordinal, TSC_ORDINAL_MAX);
#endif
                        return (0);
                }
                index = tsc_ords_duration[ordinal];
        } else {
                if (ordinal > TPM_ORDINAL_MAX) {
#ifdef DEBUG
                        cmn_err(CE_WARN,
                            "!%s: ordinal %d exceeds MAX: %d",
                            myname, ordinal, TPM_ORDINAL_MAX);
#endif
                        return (0);
                }
                index = tpm_ords_duration[ordinal];
        }

Looking at usr/src/uts/common/io/tpm_tis.h, we see:


#define TSC_ORDINAL_MAX         12
#define TPM_ORDINAL_MAX         243

and in usr/src/uts/common/io/tpm_duration.h:


static const TPM_DURATION_T tpm_ords_duration[TPM_ORDINAL_MAX] = {
...
};

...

static const uint8_t tsc_ords_duration[TSC_ORDINAL_MAX] = {
...
};

Smatch is correctly identifying a problem -- the if checks should be using >=, not > to validate the ordinal value is in bounds.

Since the tpm device has 0600 root/sys perms (i.e. it requires root access to open and use), this doesn't appear to be a security issue, but we should fix this given how trivial it is.


Related issues

Related to illumos gate - Bug #13969: tpm driver should use __func__ClosedJason King

Actions
Actions

Also available in: Atom PDF