quoted

From: Greentime Hu <redacted>

Signed-off-by: Vincent Chen <redacted>
Signed-off-by: Greentime Hu <redacted>
 arch/nds32/mm/alignment.c       |  564 +++++++++++++++++++++++++++++++++++++++

quoted

diff --git a/arch/nds32/mm/alignment.c b/arch/nds32/mm/alignment.c new

file mode 100644 index 0000000..05589e7

--- /dev/null
+++ b/arch/nds32/mm/alignment.c

quoted

+static int mode = 0x3;
+module_param(mode, int, S_IWUSR | S_IRUGO);

It's an interesting question how to best handle alignment faults, both in
kernel and user mode. While it can help for debugging to have the handler,
I'd argue that you are better off in the long run not fixing up the faults
automatically but to modify the code that triggers them instead.

How about making the faults disabled by default?

Thanks
OK, I will disable alignment fault handling by default in next version patch

quoted

+static int _do_unaligned_access(unsigned long entry, unsigned long addr,
+                               unsigned long type, struct pt_regs *regs)
+{
+       unsigned long inst;
+       int ret = -EFAULT;
+
+       if (user_mode(regs)) {
+               /* user mode */
+               if (!va_present(current->mm, addr))
+                       return ret;
+       } else {
+               /* kernel mode */
+               if (!va_kernel_present(addr))
+                       return ret;
+       }

This looks racy, the address might be present when you get here, but not
later when you actually access it. I think what you need here is something
like ARM does with get32_unaligned_check() etc and their fixup tables.

Thanks.
I will follow your suggestion to modify it

quoted

+       inst = get_inst(regs->ipc);
+
+       DEBUG(mode & 0x04, 1,
+             "Faulting Addr: 0x%08lx, PC: 0x%08lx [ 0x%08lx ]\n", addr,
+             regs->ipc, inst);
+
+       if ((user_mode(regs) && (mode & 0x01))
+           || (!user_mode(regs) && (mode & 0x02))) {
+
+               mm_segment_t seg = get_fs();
+
+               set_fs(KERNEL_DS);
+
+               if (inst & 0x80000000)
+                       ret = do_16((inst >> 16) & 0xffff, regs);
+               else
+                       ret = do_32(inst, regs);
+
+               set_fs(seg);
+       }
+
+       return ret;
+}

Doesn't this allow user space to read all of kernel memory simply by
passing unaligned addresses?

Thanks
I will fix it in next version patch.

quoted

+static const struct file_operations fops = {
+       .open = simple_open,
+       .read = proc_alignment_read,
+       .write = proc_alignment_write,
+};

This should really be a sysctl rather than an open-coded procfs file,
for consistency with other architectures.

Please have a look at that interface on other architectures and pick
whatever the majority do.

OK. I will use sysctl instaed of procfs to control this alignment
correction feature.
Thanks

    Arnd

Best regards
Vincent