| 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
| ||
|
Message-ID: <6fc308aa-a0ac-3f0e-b484-352512ad6793@huawei.com>
Date: Mon, 26 Sep 2022 21:26:50 +0800
From: Kefeng Wang <wangkefeng.wang@...wei.com>
To: "Russell King (Oracle)" <linux@...linux.org.uk>
CC: <linux-arm-kernel@...ts.infradead.org>,
<linux-kernel@...r.kernel.org>
Subject: Re: [PATCH v2] ARM: mm: Provide better fault message for permission
fault
On 2022/9/26 18:13, Russell King (Oracle) wrote:
> 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.
The __do_user_fault(), do_DataAbort() and do_PrefetchAbort() shows
the FSR when printing, we could do it in die_kernel_fault(), and
which will be easy for us to check whether the page fault is permision
fault,
--- a/arch/arm/mm/fault.c
+++ b/arch/arm/mm/fault.c
@@ -111,8 +111,8 @@ static void die_kernel_fault(const char *msg, struct
mm_struct *mm,
{
bust_spinlocks(1);
pr_alert("8<--- cut here ---\n");
- pr_alert("Unable to handle kernel %s at virtual address %08lx\n",
- msg, addr);
+ pr_alert("Unable to handle kernel %s (0x%08x) at virtual address
%08lx\n",
+ msg, fsr, addr);
show_pte(KERN_ALERT, mm, addr);
die("Oops", regs, fsr);
or,
> 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.
We could decode the FSR register,
diff --git a/arch/arm/mm/fault.c b/arch/arm/mm/fault.c
index 46cccd6bf705..406e0210c3c5 100644
--- a/arch/arm/mm/fault.c
+++ b/arch/arm/mm/fault.c
@@ -113,6 +113,10 @@ static void die_kernel_fault(const char *msg,
struct mm_struct *mm,
pr_alert("8<--- cut here ---\n");
pr_alert("Unable to handle kernel %s at virtual address %08lx\n",
msg, addr);
+ pr_alert("FSR: 0x%08x, LNX_PF = %u, CM = %u, WnR = %u\n", fsr,
+ (fsr & FSR_LNX_PF) >> FSR_LNX_PF_SHIFT,
+ (fsr & FSR_CM) >> FSR_CM_SHIFT,
+ (fsr & FSR_WRITE) >> FSR_WRITE_SHIFT);
show_pte(KERN_ALERT, mm, addr);
die("Oops", regs, fsr);
diff --git a/arch/arm/mm/fault.h b/arch/arm/mm/fault.h
index 83b5ab32d7a4..18f882aa2b32 100644
--- a/arch/arm/mm/fault.h
+++ b/arch/arm/mm/fault.h
@@ -5,9 +5,12 @@
/*
* Fault status register encodings. We steal bit 31 for our own purposes.
*/
-#define FSR_LNX_PF (1 << 31)
-#define FSR_CM (1 << 13)
-#define FSR_WRITE (1 << 11)
+#define FSR_LNX_PF_SHIFT (31)
+#define FSR_LNX_PF (1 << FSR_LNX_PF_SHIFT)
+#define FSR_CM_SHIFT (13)
+#define FSR_CM (1 << FSR_CM_SHIFT)
+#define FSR_WRITE_SHIFT (11)
+#define FSR_WRITE (1 << FSR_WRITE_SHIFT)
#define FSR_FS4 (1 << 10)
#define FSR_FS3_0 (15)
#define FSR_FS5_0 (0x3f)
What's your option ?
>
Powered by blists - more mailing lists