Bug 206755
| Summary: | Use of initialised stack variables in tdfx_query_update | ||
|---|---|---|---|
| Product: | Base System | Reporter: | CTurt <ecturt> |
| Component: | kern | Assignee: | Mahdi Mokhtari <mmokhi> |
| Status: | Closed Overcome By Events | ||
| Severity: | Affects Only Me | CC: | emaste, op, secteam, shawn.webb |
| Priority: | --- | Keywords: | patch |
| Version: | CURRENT | ||
| Hardware: | Any | ||
| OS: | Any | ||
| URL: | https://reviews.freebsd.org/D5132 | ||
Missed one of the `copyin` calls in my original patch, an additional one is needed as well: https://github.com/HardenedBSD/hardenedBSD-playground/commit/ccf98fe9312539aca1154a9462d611d8fdc4f5fa.patch Any movement on this? (In reply to Shawn Webb from comment #3) patch submitted and committed AFAIK :D (In reply to Mahdi Mokhtari from comment #4) Seems like it's still uncommitted. The patch on Phabricator is still marked as open. (In reply to Shawn Webb from comment #5) @Shawn: i see it's committed on source. Try to update your local src tree (if you have one). maybe we should close that review on Phabricator, or contact committer to do that. batch change of PRs untouched in 2018 marked "in progress" back to open. ^Triage: apparently committed long ago. |
`tdfx_query_update` in `sys/dev/tdfx/tdfx_pci.c` doesn't check the result of `copyin` calls: static int tdfx_query_update(u_int cmd, struct tdfx_pio_data *piod) { /* XXX Comment this later, after careful inspection and spring cleaning :) */ /* Return vals */ u_int8_t ret_byte; u_int16_t ret_word; u_int32_t ret_dword; ... switch (piod->size) { case 1: copyin(piod->value, &ret_byte, 1); preval = ret_byte << (8 * (piod->port & 0x3)); mask = 0xff << (8 * (piod->port & 0x3)); break; case 2: copyin(piod->value, &ret_word, 2); preval = ret_word << (8 * (piod->port & 0x3)); mask = 0xffff << (8 * (piod->port & 0x3)); break; case 4: copyin(piod->value, &ret_dword, 4); preval = ret_dword; mask = ~0; break; default: return -EINVAL; } /* Finally, combine the values and write it to the port */ retval = (retval & ~mask) | preval; pci_write_config(tdfx_info->dev, piod->port & ~3, retval, 4); If the user supplies a bad pointer, so that the `copyin` calls fail, `pci_write_config` will be passed an uninitialised stack value.