linux-kernel - Re: [PATCH] docs: Introduce deprecated APIs list

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite for Android: free password hash cracker in your pocket

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20181017065011.40ad8b24@lwn.net>
Date:   Wed, 17 Oct 2018 06:50:11 -0600
From:   Jonathan Corbet <corbet@....net>
To:     Kees Cook <keescook@...omium.org>
Cc:     linux-doc@...r.kernel.org, linux-kernel@...r.kernel.org,
        Stephen Rothwell <sfr@...b.auug.org.au>
Subject: Re: [PATCH] docs: Introduce deprecated APIs list

On Tue, 16 Oct 2018 19:17:08 -0700
Kees Cook <keescook@...omium.org> 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@...omium.org>
> ---
>  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