linux-kernel - Re: [PATCH] Documentation: x86: Fix obsolete name of page fault handler

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:   Wed, 16 Mar 2022 19:54:31 +0100
From:   Borislav Petkov <bp@...en8.de>
To:     Chin En Lin <shiyn.lin@...il.com>
Cc:     corbet@....net, tglx@...utronix.de, mingo@...hat.com,
        dave.hansen@...ux.intel.com, x86@...nel.org, hpa@...or.com,
        linux-kernel@...r.kernel.org, linux-doc@...r.kernel.org
Subject: Re: [PATCH] Documentation: x86: Fix obsolete name of page fault
 handler

On Mon, Mar 14, 2022 at 11:59:01PM +0800, Chin En Lin wrote:
> Since commit 91eeafea1e4b ("x86/entry: Switch page fault
> exception to IDTENTRY_RAW"), the function name of page
> fault handler is out of date. This patch change
> do_page_fault() to exc_page_fault().

Avoid having "This patch" or "This commit" in the commit message. It is
tautologically useless.

Also, do

$ git grep 'This patch' Documentation/process

for more details.

Also, do not talk about what your patch does - that should hopefully be
visible in the diff itself. Rather, talk about *why* you're doing what
you're doing.

> Signed-off-by: Chin En Lin <shiyn.lin@...il.com>
> ---
>  Documentation/x86/exception-tables.rst | 8 ++++----
>  1 file changed, 4 insertions(+), 4 deletions(-)
> 
> diff --git a/Documentation/x86/exception-tables.rst b/Documentation/x86/exception-tables.rst
> index de58110c5ffd..0140a06b2705 100644
> --- a/Documentation/x86/exception-tables.rst
> +++ b/Documentation/x86/exception-tables.rst
> @@ -32,14 +32,14 @@ Whenever the kernel tries to access an address that is currently not
>  accessible, the CPU generates a page fault exception and calls the
>  page fault handler::
>  
> -  void do_page_fault(struct pt_regs *regs, unsigned long error_code)
> +  void exc_page_fault(struct pt_regs *regs, unsigned long error_code)
>  
>  in arch/x86/mm/fault.c. The parameters on the stack are set up by
>  the low level assembly glue in arch/x86/entry/entry_32.S. The parameter
>  regs is a pointer to the saved registers on the stack, error_code
>  contains a reason code for the exception.
>  
> -do_page_fault first obtains the unaccessible address from the CPU
> +exc_page_fault first obtains the unaccessible address from the CPU

Unknown word [unaccessible] in Documentation.
Suggestions: ['inaccessible', 'accessible', 'accessibly']

Also, put "()" after the function name:

"exc_page_fault() first obtains..."

>  control register CR2. If the address is within the virtual address
>  space of the process, the fault probably occurred, because the page
>  was not swapped in, write protected or something similar. However,
> @@ -281,12 +281,12 @@ vma occurs?
>  
>      > c017e7a5 <do_con_write+e1> movb   (%ebx),%dl
>  #. MMU generates exception
> -#. CPU calls do_page_fault
> +#. CPU calls exc_page_fault
>  #. do page fault calls search_exception_table (regs->eip == c017e7a5);

That is not really true anymore - look at the code and see which
function calls fixup_exception() now and try to fix that too pls.

>  #. search_exception_table looks up the address c017e7a5 in the
>     exception table (i.e. the contents of the ELF section __ex_table)
>     and returns the address of the associated fault handle code c0199ff5.
> -#. do_page_fault modifies its own return address to point to the fault
> +#. exc_page_fault modifies its own return address to point to the fault
>     handle code and returns.

Ditto.

Thx.

-- 
Regards/Gruss,
    Boris.

https://people.kernel.org/tglx/notes-about-netiquette