Bug 206755 - Use of initialised stack variables in tdfx_query_update
Summary: Use of initialised stack variables in tdfx_query_update
Status: Open
Alias: None
Product: Base System
Classification: Unclassified
Component: kern (show other bugs)
Version: CURRENT
Hardware: Any Any
: --- Affects Only Me
Assignee: Mahdi Mokhtari
URL: https://reviews.freebsd.org/D5132
Keywords: patch
Depends on:
Blocks:
 
Reported: 2016-01-30 09:53 UTC by CTurt
Modified: 2018-05-23 10:27 UTC (History)
5 users (show)

See Also:


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description CTurt 2016-01-30 09:53:38 UTC
`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.
Comment 2 CTurt 2016-01-30 10:06:12 UTC
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
Comment 3 Shawn Webb 2016-03-01 20:59:43 UTC
Any movement on this?
Comment 4 Mahdi Mokhtari freebsd_committer freebsd_triage 2016-03-02 11:09:11 UTC
(In reply to Shawn Webb from comment #3)
patch submitted and committed AFAIK :D
Comment 5 Shawn Webb 2016-03-02 12:59:41 UTC
(In reply to Mahdi Mokhtari from comment #4)

Seems like it's still uncommitted. The patch on Phabricator is still marked as open.
Comment 6 Mahdi Mokhtari freebsd_committer freebsd_triage 2016-03-02 13:19:04 UTC
(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.
Comment 7 Eitan Adler freebsd_committer freebsd_triage 2018-05-23 10:27:41 UTC
batch change of PRs untouched in 2018 marked "in progress" back to open.