linux-kernel - Re: [PATCH -next v2 1/2] riscv: uaccess: rename __get/put_user_nocheck to __get/put_mem

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: Windows password security audit tool. GUI, reports in PDF.

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

Date:   Fri, 26 Aug 2022 11:30:26 +0200
From:   Arnd Bergmann <arnd@...db.de>
To:     Tong Tiangen <tongtiangen@...wei.com>
Cc:     Paul Walmsley <paul.walmsley@...ive.com>,
        Palmer Dabbelt <palmer@...belt.com>,
        Palmer Dabbelt <palmer@...osinc.com>,
        Albert Ou <aou@...s.berkeley.edu>, Conor.Dooley@...rochip.com,
        linux-riscv@...ts.infradead.org, linux-kernel@...r.kernel.org,
        wangkefeng.wang@...wei.com, Guohanjun <guohanjun@...wei.com>
Subject: Re: [PATCH -next v2 1/2] riscv: uaccess: rename __get/put_user_nocheck
 to __get/put_mem_nocheck

On Mon, Aug 15, 2022 at 5:20 AM Tong Tiangen <tongtiangen@...wei.com> wrote:
>
> Current, The helpers __get/put_user_nocheck() is used by get/put_user() and
> __get/put_kernel_nofault(), which is not always uaccess, so the name with
> *user* is not appropriate.
>
> Also rename xxx_user_xxx to xxx_mem_xx  on the call path of
> __get/put_user_nocheck()
>
> Only refactor code without any functional changes.
>
> Signed-off-by: Tong Tiangen <tongtiangen@...wei.com>

I would prefer this not being done, it just makes riscv diverge from the
code on other architectures. While the new name does make more sense,
it ends up making it harder to refactor this across architectures in the end.

There are two important cleanups that I would like to see done in
asm/uaccess.h across architectures:

 - generalize the __get_user()/__put_user()/__get_kernel_nofault()/
  __put_kernel_nofault() wrappers to the point that architectures do not
  need to worry about the variable type stuff but instead just provide
  trivial fixed-length helpers of some sort

- change the calling conventions in a way that allows the use of the
  asm-goto-with-output method for better object code on modern
  compilers.

The x86 version already has most of this, with their
__get_user_size() macro supporting both the asm-goto label
and the error code assignment, so the generalized code should
probably be based on that approach.

       Arnd