From: Kees Cook <keescook@chromium.org>
To: ksummit <ksummit-discuss@lists.linuxfoundation.org>
Subject: [Ksummit-discuss] [MAINTAINERS SUMMIT] API replacement/deprecation
Date: Wed, 5 Sep 2018 15:57:02 -0700 [thread overview]
Message-ID: <CAGXu5j+-X6WQNdTYK2ZA5OOJgc0RfCq9Gr9WRAMkYTSQOT7QdQ@mail.gmail.com> (raw)
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
next reply other threads:[~2018-09-05 22:57 UTC|newest]
Thread overview: 39+ messages / expand[flat|nested] mbox.gz Atom feed top
2018-09-05 22:57 Kees Cook [this message]
2018-09-05 23:41 ` [Ksummit-discuss] [MAINTAINERS SUMMIT] API replacement/deprecation Stephen Rothwell
2018-09-06 2:24 ` Steven Rostedt
2018-09-06 6:12 ` Julia Lawall
2018-09-06 18:24 ` Kees Cook
2018-09-06 23:18 ` Stephen Rothwell
2018-09-06 23:24 ` Kees Cook
2018-09-07 7:03 ` Takashi Iwai
2018-09-07 7:20 ` Johannes Berg
2018-09-07 7:31 ` Takashi Iwai
2018-09-07 9:42 ` Julia Lawall
2018-09-07 8:04 ` Jani Nikula
2018-09-07 9:38 ` Julia Lawall
2018-09-07 9:54 ` Jani Nikula
2018-09-07 10:05 ` Julia Lawall
2018-09-07 10:43 ` Jani Nikula
2018-09-07 10:25 ` Alexandre Belloni
2018-09-07 11:44 ` Mark Brown
2018-09-10 12:51 ` Mauro Carvalho Chehab
2018-09-11 8:10 ` Jani Nikula
2018-09-11 9:34 ` Mauro Carvalho Chehab
2018-09-11 11:08 ` Arnd Bergmann
2018-09-07 8:19 ` Jan Kara
2018-09-07 14:33 ` Theodore Y. Ts'o
2018-09-07 16:10 ` Kees Cook
2018-09-07 20:30 ` Arnd Bergmann
2018-09-07 20:56 ` Theodore Y. Ts'o
2018-09-08 8:15 ` Geert Uytterhoeven
2018-09-08 15:19 ` Theodore Y. Ts'o
2018-09-10 12:28 ` Mauro Carvalho Chehab
2018-09-10 16:09 ` Kees Cook
2018-09-07 10:14 ` Dan Carpenter
2018-09-07 10:40 ` Geert Uytterhoeven
2018-09-07 8:40 ` Maxime Ripard
2018-09-06 4:44 ` Julia Lawall
2018-09-06 10:04 ` Linus Walleij
2018-09-06 10:11 ` Geert Uytterhoeven
2018-09-06 14:59 ` Kees Cook
2018-09-06 15:06 ` Geert Uytterhoeven
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=CAGXu5j+-X6WQNdTYK2ZA5OOJgc0RfCq9Gr9WRAMkYTSQOT7QdQ@mail.gmail.com \
--to=keescook@chromium.org \
--cc=ksummit-discuss@lists.linuxfoundation.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).