Actions
Bug #13968
closedtpm driver fails bounds checks
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
Actions