re: CVS commit: src/sys/dev/pci

[Paul Goyette <paul%whooppee.com@localhost>]

On Wed, 26 Oct 2016, matthew green wrote:

> i think you're right that the 'cp' manipulation is the problem.
> snprintf() will return the "desired" size, so upon the first
> attempted overflow the 'cp' is moved beyond 'ep', and then the
> next snprintf() gets a negative aka extremely massive value
> for the buffer length and actual overflow happens here, and ssp
> detects it.
>
> the fix would be to change this:
>
> 	cp += snprintf(cp, ep - cp, ...);
>
> into this:
>
> 	len = snprintf(cp, ep - cp, ...);
> 	if (len > ep - cp)
> 		return;
> 	cp += len;
>
> which is annoying because there are a lot of the former.

There's only 9 snprintf() calls.  I could simply provide a macro:

#define ADD_TEXT(dest, end, format, ...)			\
	{							\
		int len, max = (end) - (dest);			\

		len = snprintf((dest), max, (format), __VA_ARGS__); \
		if (len > max)					\
			return;					\
		(dest) += len;					\
	}


Then all of the snprintf() calls become simply

	ADD_TEXT(cp, ep, <format>, ...);

(Of course, after last use I'd add a #undef ADD_TEXT to clean up...)



+------------------+--------------------------+------------------------+
| Paul Goyette     | PGP Key fingerprint:     | E-mail addresses:      |
| (Retired)        | FA29 0E3B 35AF E8AE 6651 | paul at whooppee.com   |
| Kernel Developer | 0786 F758 55DE 53BA 7731 | pgoyette at netbsd.org |
+------------------+--------------------------+------------------------+