On Thu, Nov 17, 2011 at 10:47:01AM -0500, Thor Lancelot Simon wrote: > Here is my concern: I think syntax and semantics are being confused here > in a way that makes it likely we will have a false sense of security > about bugs that could crash the kernel (or lead to severe security > issues, etc). > > I do not see how proplib helps at all with bugs caused by syntactically > valid data with semantically invalid values. This means that the > kernel side code must parse the property list, then check each > value sufficiently to ensure that using those values will not cause > an error (at worst, a crash). > > Consider the (very poor) old interface, in which void pointers were > thrown around. Suppose they pointed at bad addresses? No system call > should directly dereference pointers passed by userspace, so the user > application should get EFAULT if it passes something that can't copy-in, > end of game. > > But suppose that void* actually points at valid user memory that has > insane contents when interpreted as a quota structure? In that case, I > cannot see why the validity checks required are any more, nor any less, > than those required for the parsed data from the proplib interface. The > fields have acceptable values, or they don't; if they have insane values > and those values are just used by the kernel and cause a crash, that > consequence should be the same either way -- whether the insane values > came from a property list or from a binary datastructure. > > What am I missing?
the quotactl call has different commands, which takes different arguments. For example, quotaon takes a string, while setquota takes a struct describing the quotas to be set. With the old quotactl, the kernel has no way to tell if it really got a string of a struct dqblk (or something else); it can just interpret the pointer as a string and see if it works. If instead of a string it got a dqblk whose first 2 entries happends to be 0x6f6f622f 0x78300074, you'll have troubles (you're doing quotaon on /boot). With a proplib format, the kernel knows it didn't get the right argument (it didn't find a key "quotafile" with a string value in the dictionary). Of course you can still do quotaon /boot if you really wants to, but then it has been done on purpose, not just because you gave the wrong pointer to quotactl(). -- Manuel Bouyer <bou...@antioche.eu.org> NetBSD: 26 ans d'experience feront toujours la difference --