Re: [PATCH] docs: Introduce deprecated APIs list

[Jonathan Corbet]

On Tue, 16 Oct 2018 19:17:08 -0700
Kees Cook <keescook@xxxxxxxxxxxx> wrote:

> As discussed in the "API replacement/deprecation" thread[1], this
> makes an effort to document what things shouldn't get (re)added to the
> kernel, by introducing Documentation/process/deprecated.rst. It also
> adds the overflow kerndoc to ReST output, and tweaks the struct_size()
> documentation to parse correctly.

Seems like a good idea overall...a couple of quick comments

> [1] https://lists.linuxfoundation.org/pipermail/ksummit-discuss/2018-September/005282.html
> 
> Signed-off-by: Kees Cook <keescook@xxxxxxxxxxxx>
> ---
>  Documentation/driver-api/basics.rst  |  3 +
>  Documentation/process/deprecated.rst | 99 ++++++++++++++++++++++++++++
>  Documentation/process/index.rst      |  1 +

I wonder if "process" is the right place, or core-api?  I guess we have a
lot of similar stuff in process now.

[...]

> +open-coded arithmetic in allocator arguments
> +--------------------------------------------
> +Dynamic size calculations (especially multiplication)
> +should not be performed in memory allocator (or similar)
> +function arguments.
> +
> +For example, do not use ``rows * cols`` as an argument, as in:
> +``kmalloc(rows * cols, GFP_KERNEL)``.
> +Instead, the 2-factor form of the allocator should be used:
> +``kmalloc_array(rows, cols, GFP_KERNEL)``.
> +If no 2-factor form is available, the saturate-on-overflow helpers should
> +be used:
> +``vmalloc(array_size(rows, cols))``.
> +
> +See :c:func:`array_size`, :c:func:`array3_size`, and :c:func:`struct_size`,
> +for more details as well as the related :c:func:`check_add_overflow` and
> +:c:func:`check_mul_overflow` family of functions.

I think this should say *why* developers are being told not to do it.  Does
this advice hold even in cases where the dimensions are known, under the
kernel's control, and guaranteed not to overflow even on Alan's port to
eight-bit CPUs?

To me it's also a bit confusing to present the arguments to kmalloc_array()
as "rows" and "cols" when they are really "n" and "size".

Thanks,

jon