Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1478516

Re: [PATCH] usercopy: remove page-spanning test for now

Show all headers | View raw

On Wed, Sep 7, 2016 at 10:06 AM, Kees Cook <keescook@chromium.org> wrote:
>
> +#ifndef CONFIG_HARDENED_USERCOPY_PAGESPAN
> +       /*
> +        * The page-spanning checks are hitting false positives, so
> +        * do not check them for now.
> +        */
> +       return NULL;
> +#endif
> +
>         /* Allow kernel data region (if not marked as Reserved). */
>         if (ptr >= (const void *)_sdata && end <= (const void *)_edata)
>                 return NULL;

No. Don't do patches like this.

It's wrong for two reasons:

 (a) if you want to use an #ifdef to disable code, do so. Enclose the
code you want to disable with the #ifdef, not some *other* code that
then indirectly disables the code you want to disable.

 (b) don't do "surprising" things with control flow. It can cause
compiler warnings in reasonable compilers ("unreachable code"), but
it's also a strange pattern that throws people.

So really, make the patch bigger but more legible. In fact, I think
the best option would be to simply turn the code you want to disable
into a helper function of its own, and then make the #ifdef enable or
disable the whole function.

(That also solves the problems like having the declaration for
"endpage" and the other variables that is only used by the disabled
code be *with* the disabled code, so that you don't have to have
multiple ifdef'ed regions. I suspect avoiding that is a large reason
why you did the hacky thing in the first place).

                Linus

Back to linux.kernel | Previous | Next — Previous in thread | Next in thread | Find similar | Unroll thread

Thread

[PATCH] usercopy: remove page-spanning test for now Kees Cook <keescook@chromium.org> - 2016-09-07 19:10 +0200
  Re: [PATCH] usercopy: remove page-spanning test for now Linus Torvalds <torvalds@linux-foundation.org> - 2016-09-07 19:20 +0200
    Re: [PATCH] usercopy: remove page-spanning test for now Vegard Nossum <vegard.nossum@gmail.com> - 2016-09-07 22:40 +0200

csiph-web