Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
Date:   Mon, 26 Sep 2022 11:13:51 +0100
From:   "Russell King (Oracle)" <linux@armlinux.org.uk>
To:     Kefeng Wang <wangkefeng.wang@huawei.com>
Cc:     linux-arm-kernel@lists.infradead.org, linux-kernel@vger.kernel.org
Subject: Re: [PATCH v2] ARM: mm: Provide better fault message for permission
 fault
Message-ID: <YzF7X2PBdps2MaG/@shell.armlinux.org.uk>
References: <20220919103845.100809-1-wangkefeng.wang@huawei.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <20220919103845.100809-1-wangkefeng.wang@huawei.com>
Sender: Russell King (Oracle) <linux@armlinux.org.uk>
Precedence: bulk

On Mon, Sep 19, 2022 at 06:38:45PM +0800, Kefeng Wang wrote:
> If there is a permission fault in __do_kernel_fault(), we only
> print the generic "paging request" message which don't show
> read, write or excute information, let's provide better fault
> message for them.

I don't like this change. With CPUs that do not have the ability to
relocate the vectors to 0xffff0000, the vectors live at address 0,
so NULL pointer dereferences can produce permission faults.

I would much rather we did something similar to what x86 does:

        pr_alert("#PF: %s %s in %s mode\n",
                 (error_code & X86_PF_USER)  ? "user" : "supervisor",
                 (error_code & X86_PF_INSTR) ? "instruction fetch" :
                 (error_code & X86_PF_WRITE) ? "write access" :
                                               "read access",
                             user_mode(regs) ? "user" : "kernel");

As we already print whether we're in user or kernel mode in the
register dump, there's no need to repeat that. I think we just
need an extra line to decode the FSR PF and write bits.

-- 
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTP is here! 40Mbps down 10Mbps up. Decent connectivity at last!