[Ksummit-discuss] [MAINTAINERS SUMMIT] API replacement/deprecation

[Kees Cook]

Hi,

I'd like to discuss ways that we could deprecate APIs more sanely. At
present I've seen (and used) two approaches, fast and slow:

Fast:
Introduce new API in release N. Perform massive patch bombs drowning
maintainers in a single release for release N+1, and remove the old
API before -rc2.
Pros:
- it gets done in 2 releases!
Cons:
- writing the API transition patches may span multiple release prior
to N without entering linux-next, which causes rebase/conflict cycles
- patches developed prior to N may not get adequate testing
- maintainers drown in patches
Example: timer_setup() refactoring, kmalloc()->kmalloc_array()

Slow:
Introduce new API in release N. Trickle patches in over N+M releases,
removing old API when no more uses are present.
Pros:
- patches are written and handed to maintainers at a regular rate
- testing happens normally
Cons:
- new users of the old API continue to enter the kernel, causing an
endless cycle of needing more work
- API may never get deprecated
Example: strscpy(), VLA removal

Note that "Fast" isn't actually fast, since the bulk of the prep work
happens "in the dark".

The primary issue with "Slow" is the introduction of uses of the old
API. With a "small" API, this is managemable (e.g. with VLA removal we
started with only about 120 instances, and only 2-3 new ones got added
each release). With large mechanical changes (kmalloc_array()) it's
easy to keep up with new users, since the fixes are mechanical.

While trying to prepare to move away from bare strcpy(),
not-always-terminated strncpy(), and
read-entire-src-and-do-full-NUL-padding strlen(), there are literally
thousands of users to move to strscpy(), and I expect new users to
keep entering the kernel at a high rate. Adding warnings to
checkpatch.pl isn't sufficient since only a subset of maintainers
require patches be "checkpatch clean".

Some str*cpy() numbers for context:
$ for i in '' n l s; do j=str${i}cpy; echo -en "$j\t"; git grep
'\b'"$j"'\b' | grep -Ev '^(Documentation|tools)/' | wc -l ; done
strcpy  2490
strncpy 865
strlcpy 2361
strscpy 29

Linus correctly removed __deprecated since it's just noisy: marking
strcpy() as __deprecated would be an absolute nightmare. What would be
great would be a way to mark NEW users of an API with a warning, but
outside of voluntary checkpatch use, we have no global patch checker
that is always run. Adding some kind of "__grandfathered" mark to
existing users that would silence a __deprecated warning seems like
just creating two problems, and would create massive churn in the
existing code.

I'd really like a solution besides "masochism". :)

-Kees

-- 
Kees Cook
Pixel Security