tech-security archive

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

Re: strscpy



> Date: Tue, 2 Jun 2020 20:25:45 +0200
> From: Maxime Villard <max%m00nbsd.net@localhost>
> 
> Here is a patch that converts all of the copystr() calls in the kernel to
> strkcpy(). One exception is vfs_subr.c where it will be done differently
> and isn't included here.
> 
> 	https://m00nbsd.net/garbage/libkern/strkcpy.diff
> 
> This covers the "difficult" cases. The rest is easy and can be switched
> almost mechanically.

Replacing copystr by your strkcpy strikes me as the opposite of an
improvement -- it would replace nearly all of the calls to copystr by
logic that is more complicated, not parallel with the nearby copyinstr
logic, and harder to audit.

Trying to usefully merge the error indicator with the returned length
seems increasingly dubious to me:

- strlcpy is consistent about the meaning of the result but the result
  is usually useless and scans past what it needs in the input string

- returning either len or len+1 is confusing because the meaning is
  _sometimes_ strlen of the output but _sometimes_ the size you need
  to memcpy to get the same C string as the output

- returning -1 requires ssize_t and annoying casts and if _added_ to
  something doesn't work very well

- returning SIZE_MAX is same as -1 without the casts

- returning SSIZE_MAX leaves questions about large copies in 32-bit
  address spaces

I would like to see an assessment of how it improves the majority of
cases, not just how to use it in the handful of difficult edge cases
you've identified.


Home | Main Index | Thread Index | Old Index