Source-Changes-D archive

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index][Old Index]

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



On Wed, 26 Oct 2016, Paul Goyette wrote:

HOWEVER,

Looking at dev_untokenstring() it would appear that there is a potential problem there! We have

               cp = buf + strlcat(buf, words + *token, len - 2);
               cp[0] = ' ';
               cp[1] = '\0';


Since strlcat(3) (quoting the man page) "return[s] the total length of the string [it] tried to create", it is entirely possible for cp to point beyond the end of the buffer. Thus the insertion of a trailing space-between-word and the NUL character could occur on some random location.

It would seem to me the this code should be written as

		newlen = strlcat(buf, word + *token, len - 2);
		if (newlen > len - 2)
			newlen = len - 2;
		cp = buf + newlen;
		cp[0] = ' ';
		cp[1] = '\0';

I have confirmed that this actually is the root cause of the stack overflow problem.

With this fix, and the original buffer size of 64, I get a truncated product description (as expected), but the stack overflow is gone.

With this fix and the updated buffer size, I get the full description without any overflow.

With the original code (smaller buffer, above fix not included) I got consistent stack overflow instead of proper printing of the product description.

The above fix has been committed.


+------------------+--------------------------+------------------------+
| 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 |
+------------------+--------------------------+------------------------+


Home | Main Index | Thread Index | Old Index