| Message ID | fb70dc86ccb3f0e062c25c81d948171d8534ee63.1600204505.git.andreyknvl@google.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | kasan: add hardware tag-based mode for arm64 | expand |
On Tue, Sep 15, 2020 at 11:16:16PM +0200, Andrey Konovalov wrote: > diff --git a/arch/arm64/mm/fault.c b/arch/arm64/mm/fault.c > index cdc23662691c..ac79819317f2 100644 > --- a/arch/arm64/mm/fault.c > +++ b/arch/arm64/mm/fault.c > @@ -14,6 +14,7 @@ > #include <linux/mm.h> > #include <linux/hardirq.h> > #include <linux/init.h> > +#include <linux/kasan.h> > #include <linux/kprobes.h> > #include <linux/uaccess.h> > #include <linux/page-flags.h> > @@ -295,17 +296,23 @@ static void die_kernel_fault(const char *msg, unsigned long addr, > do_exit(SIGKILL); > } > > +#ifdef CONFIG_KASAN_HW_TAGS > static void report_tag_fault(unsigned long addr, unsigned int esr, > struct pt_regs *regs) > { > - bool is_write = ((esr & ESR_ELx_WNR) >> ESR_ELx_WNR_SHIFT) != 0; > + bool is_write = ((esr & ESR_ELx_WNR) >> ESR_ELx_WNR_SHIFT) != 0; > > - pr_alert("Memory Tagging Extension Fault in %pS\n", (void *)regs->pc); > - pr_alert(" %s at address %lx\n", is_write ? "Write" : "Read", addr); > - pr_alert(" Pointer tag: [%02x], memory tag: [%02x]\n", > - mte_get_ptr_tag(addr), > - mte_get_mem_tag((void *)addr)); > + /* > + * SAS bits aren't set for all faults reported in EL1, so we can't > + * find out access size. > + */ > + kasan_report(addr, 0, is_write, regs->pc); > } > +#else > +/* Tag faults aren't enabled without CONFIG_KASAN_HW_TAGS. */ > +static inline void report_tag_fault(unsigned long addr, unsigned int esr, > + struct pt_regs *regs) { } > +#endif So is there a point in introducing this function in an earlier patch, just to remove its content here?
On Thu, Sep 17, 2020 at 7:04 PM Catalin Marinas <catalin.marinas@arm.com> wrote: > > On Tue, Sep 15, 2020 at 11:16:16PM +0200, Andrey Konovalov wrote: > > diff --git a/arch/arm64/mm/fault.c b/arch/arm64/mm/fault.c > > index cdc23662691c..ac79819317f2 100644 > > --- a/arch/arm64/mm/fault.c > > +++ b/arch/arm64/mm/fault.c > > @@ -14,6 +14,7 @@ > > #include <linux/mm.h> > > #include <linux/hardirq.h> > > #include <linux/init.h> > > +#include <linux/kasan.h> > > #include <linux/kprobes.h> > > #include <linux/uaccess.h> > > #include <linux/page-flags.h> > > @@ -295,17 +296,23 @@ static void die_kernel_fault(const char *msg, unsigned long addr, > > do_exit(SIGKILL); > > } > > > > +#ifdef CONFIG_KASAN_HW_TAGS > > static void report_tag_fault(unsigned long addr, unsigned int esr, > > struct pt_regs *regs) > > { > > - bool is_write = ((esr & ESR_ELx_WNR) >> ESR_ELx_WNR_SHIFT) != 0; > > + bool is_write = ((esr & ESR_ELx_WNR) >> ESR_ELx_WNR_SHIFT) != 0; > > > > - pr_alert("Memory Tagging Extension Fault in %pS\n", (void *)regs->pc); > > - pr_alert(" %s at address %lx\n", is_write ? "Write" : "Read", addr); > > - pr_alert(" Pointer tag: [%02x], memory tag: [%02x]\n", > > - mte_get_ptr_tag(addr), > > - mte_get_mem_tag((void *)addr)); > > + /* > > + * SAS bits aren't set for all faults reported in EL1, so we can't > > + * find out access size. > > + */ > > + kasan_report(addr, 0, is_write, regs->pc); > > } > > +#else > > +/* Tag faults aren't enabled without CONFIG_KASAN_HW_TAGS. */ > > +static inline void report_tag_fault(unsigned long addr, unsigned int esr, > > + struct pt_regs *regs) { } > > +#endif > > So is there a point in introducing this function in an earlier patch, > just to remove its content here? I added it to make the first patch somewhat self-consistent. But we can drop it in v3 if you think it's not needed.
diff --git a/arch/arm64/mm/fault.c b/arch/arm64/mm/fault.c index cdc23662691c..ac79819317f2 100644 --- a/arch/arm64/mm/fault.c +++ b/arch/arm64/mm/fault.c @@ -14,6 +14,7 @@ #include <linux/mm.h> #include <linux/hardirq.h> #include <linux/init.h> +#include <linux/kasan.h> #include <linux/kprobes.h> #include <linux/uaccess.h> #include <linux/page-flags.h> @@ -295,17 +296,23 @@ static void die_kernel_fault(const char *msg, unsigned long addr, do_exit(SIGKILL); } +#ifdef CONFIG_KASAN_HW_TAGS static void report_tag_fault(unsigned long addr, unsigned int esr, struct pt_regs *regs) { - bool is_write = ((esr & ESR_ELx_WNR) >> ESR_ELx_WNR_SHIFT) != 0; + bool is_write = ((esr & ESR_ELx_WNR) >> ESR_ELx_WNR_SHIFT) != 0; - pr_alert("Memory Tagging Extension Fault in %pS\n", (void *)regs->pc); - pr_alert(" %s at address %lx\n", is_write ? "Write" : "Read", addr); - pr_alert(" Pointer tag: [%02x], memory tag: [%02x]\n", - mte_get_ptr_tag(addr), - mte_get_mem_tag((void *)addr)); + /* + * SAS bits aren't set for all faults reported in EL1, so we can't + * find out access size. + */ + kasan_report(addr, 0, is_write, regs->pc); } +#else +/* Tag faults aren't enabled without CONFIG_KASAN_HW_TAGS. */ +static inline void report_tag_fault(unsigned long addr, unsigned int esr, + struct pt_regs *regs) { } +#endif static void __do_kernel_fault(unsigned long addr, unsigned int esr, struct pt_regs *regs) diff --git a/mm/kasan/report.c b/mm/kasan/report.c index c904edab33b8..34ef81736d73 100644 --- a/mm/kasan/report.c +++ b/mm/kasan/report.c @@ -65,9 +65,14 @@ static void print_error_description(struct kasan_access_info *info) { pr_err("BUG: KASAN: %s in %pS\n", get_bug_type(info), (void *)info->ip); - pr_err("%s of size %zu at addr %px by task %s/%d\n", - info->is_write ? "Write" : "Read", info->access_size, - info->access_addr, current->comm, task_pid_nr(current)); + if (info->access_size) + pr_err("%s of size %zu at addr %px by task %s/%d\n", + info->is_write ? "Write" : "Read", info->access_size, + info->access_addr, current->comm, task_pid_nr(current)); + else + pr_err("%s at addr %px by task %s/%d\n", + info->is_write ? "Write" : "Read", + info->access_addr, current->comm, task_pid_nr(current)); } static DEFINE_SPINLOCK(report_lock);