diff for duplicates of <1536958012.12990.14.camel@intel.com> diff --git a/a/content_digest b/N1/content_digest index 22102a0..b9705a8 100644 --- a/a/content_digest +++ b/N1/content_digest @@ -29,7 +29,12 @@ Kees Cook <keescook@chromium.org> Mike Kravetz <mike.kravetz@oracle.com> Nadav Amit <nadav.amit@gmail.com> - " Oleg Nesterov <oleg@redhat.>\0" + Oleg Nesterov <oleg@redhat.com> + Pavel Machek <pavel@ucw.cz> + Peter Zijlstra <peterz@infradead.org> + Ravi V. Shankar <ravi.v.shankar@intel.com> + Shanbhogue + " Vedvyas <vedvyas.shanbhogue@intel.com>\0" "\00:1\0" "b\0" "On Fri, 2018-08-31 at 15:16 -0700, Andy Lutomirski wrote:\n" @@ -145,4 +150,4 @@ "+\treturn -1;\n" +} -f1c329bc5d3b123c96aa462543811f4f526b31711be4e2c610ea41315a116a80 +409378e37136d9f02ad54b5ba715869867ef0bf64bdc1264a04a3f1ea96ec31f
diff --git a/a/1.txt b/N2/1.txt index b7577f6..0034b07 100644 --- a/a/1.txt +++ b/N2/1.txt @@ -19,12 +19,12 @@ On Fri, 2018-08-31 at 15:16 -0700, Andy Lutomirski wrote: > > > > > > > > > > > > > > > > > > WRUSS is a new kernel-mode instruction but writes directly -> > > > > > to user shadow stack memory. This is used to construct +> > > > > > to user shadow stack memory.A A This is used to construct > > > > > > a return address on the shadow stack for the signal > > > > > > handler. > > > > > > > > > > > > This instruction can fault if the user shadow stack is -> > > > > > invalid shadow stack memory. In that case, the kernel does +> > > > > > invalid shadow stack memory.A A In that case, the kernel does > > > > > > fixup. > > > > > > > > > > > > Signed-off-by: Yu-cheng Yu <yu-cheng.yu@intel.com> @@ -35,16 +35,16 @@ On Fri, 2018-08-31 at 15:16 -0700, Andy Lutomirski wrote: > > > > > > +static inline int write_user_shstk_64(unsigned long addr, > > > > > > unsigned long val) > > > > > > +{ -> > > > > > + int err = 0; +> > > > > > +A A A A A A A int err = 0; > > > > > > + -> > > > > > + asm volatile("1: wrussq %1, (%0)\n" -> > > > > > + "2:\n" -> > > > > > + _ASM_EXTABLE_HANDLE(1b, 2b, +> > > > > > +A A A A A A A asm volatile("1: wrussq %1, (%0)\n" +> > > > > > +A A A A A A A A A A A A A A A A A A A A "2:\n" +> > > > > > +A A A A A A A A A A A A A A A A A A A A _ASM_EXTABLE_HANDLE(1b, 2b, > > > > > > ex_handler_wruss) -> > > > > > + : -> > > > > > + : "r" (addr), "r" (val)); +> > > > > > +A A A A A A A A A A A A A A A A A A A A : +> > > > > > +A A A A A A A A A A A A A A A A A A A A : "r" (addr), "r" (val)); > > > > > > + -> > > > > > + return err; +> > > > > > +A A A A A A A return err; > > > > > > +} > > > > > What's up with "err"? You set it to zero, and then you return > > > > > it, @@ -56,12 +56,12 @@ On Fri, 2018-08-31 at 15:16 -0700, Andy Lutomirski wrote: > > > > > > > > > > > > +__visible bool ex_handler_wruss(const struct > > > > > > exception_table_entry *fixup, -> > > > > > + struct pt_regs *regs, int +> > > > > > +A A A A A A A A A A A A A A A A A A A A A A A A A A A A A A A struct pt_regs *regs, int > > > > > > trapnr) > > > > > > +{ -> > > > > > + regs->ip = ex_fixup_addr(fixup); -> > > > > > + regs->ax = -1; -> > > > > > + return true; +> > > > > > +A A A A A A A regs->ip = ex_fixup_addr(fixup); +> > > > > > +A A A A A A A regs->ax = -1; +> > > > > > +A A A A A A A return true; > > > > > > +} > > > > > And here you just write into regs->ax, but your "asm volatile" > > > > > doesn't @@ -69,32 +69,32 @@ On Fri, 2018-08-31 at 15:16 -0700, Andy Lutomirski wrote: > > > > > > > > > > I think you probably want to add something like an explicit > > > > > `"+&a"(err)` output to the asm statements. -> > > > We require asm goto support these days. How about using -> > > > that? You +> > > > We require asm goto support these days.A A How about using +> > > > that?A A You > > > > won't even need a special exception handler. -> > Maybe something like this? It looks simple now. +> > Maybe something like this?A A It looks simple now. > > > > static inline int write_user_shstk_64(unsigned long addr, unsigned > > long val) > > { -> > asm_volatile_goto("wrussq %1, (%0)\n" -> > "jmp %l[ok]\n" -> > ".section .fixup,\"ax\"n" -> > "jmp %l[fail]\n" -> > ".previous\n" -> > :: "r" (addr), "r" (val) -> > :: ok, fail); +> > A A A A A A A A asm_volatile_goto("wrussq %1, (%0)\n" +> > A A A A A A A A A A A A A A A A A A A A A "jmp %l[ok]\n" +> > A A A A A A A A A A A A A A A A A A A A A ".section .fixup,\"ax\"n" +> > A A A A A A A A A A A A A A A A A A A A A "jmp %l[fail]\n" +> > A A A A A A A A A A A A A A A A A A A A A ".previous\n" +> > A A A A A A A A A A A A A A A A A A A A A :: "r" (addr), "r" (val) +> > A A A A A A A A A A A A A A A A A A A A A :: ok, fail); > > ok: -> > return 0; +> > A A A A A A A A return 0; > > fail: -> > return -1; +> > A A A A A A A A return -1; > > } > > > I think you can get rid of 'jmp %l[ok]' and the ok label and just fall -> through. And you don't need an explicit jmp to fail -- just set the +> through.A A And you don't need an explicit jmp to fail -- just set the > _EX_HANDLER entry to land on the fail label. -Thanks! This now looks simple and much better. +Thanks! A This now looks simple and much better. Yu-cheng @@ -103,9 +103,9 @@ Yu-cheng +static inline int write_user_shstk_64(unsigned long addr, unsigned long val) +{ + asm_volatile_goto("1: wrussq %1, (%0)\n" -+ _ASM_EXTABLE(1b, %l[fail]) -+ :: "r" (addr), "r" (val) -+ :: fail); ++ A A _ASM_EXTABLE(1b, %l[fail]) ++ A A :: "r" (addr), "r" (val) ++ A A :: fail); + return 0; +fail: + return -1; diff --git a/a/content_digest b/N2/content_digest index 22102a0..2f78af3 100644 --- a/a/content_digest +++ b/N2/content_digest @@ -29,7 +29,12 @@ Kees Cook <keescook@chromium.org> Mike Kravetz <mike.kravetz@oracle.com> Nadav Amit <nadav.amit@gmail.com> - " Oleg Nesterov <oleg@redhat.>\0" + Oleg Nesterov <oleg@redhat.com> + Pavel Machek <pavel@ucw.cz> + Peter Zijlstra <peterz@infradead.org> + Ravi V. Shankar <ravi.v.shankar@intel.com> + Shanbhogue + " Vedvyas <vedvyas.shanbhogue@intel.com>\0" "\00:1\0" "b\0" "On Fri, 2018-08-31 at 15:16 -0700, Andy Lutomirski wrote:\n" @@ -53,12 +58,12 @@ "> > > > > > \n" "> > > > > > \n" "> > > > > > WRUSS is a new kernel-mode instruction but writes directly\n" - "> > > > > > to user shadow stack memory.\302\240\302\240This is used to construct\n" + "> > > > > > to user shadow stack memory.A A This is used to construct\n" "> > > > > > a return address on the shadow stack for the signal\n" "> > > > > > handler.\n" "> > > > > > \n" "> > > > > > This instruction can fault if the user shadow stack is\n" - "> > > > > > invalid shadow stack memory.\302\240\302\240In that case, the kernel does\n" + "> > > > > > invalid shadow stack memory.A A In that case, the kernel does\n" "> > > > > > fixup.\n" "> > > > > > \n" "> > > > > > Signed-off-by: Yu-cheng Yu <yu-cheng.yu@intel.com>\n" @@ -69,16 +74,16 @@ "> > > > > > +static inline int write_user_shstk_64(unsigned long addr,\n" "> > > > > > unsigned long val)\n" "> > > > > > +{\n" - "> > > > > > +\302\240\302\240\302\240\302\240\302\240\302\240\302\240int err = 0;\n" + "> > > > > > +A A A A A A A int err = 0;\n" "> > > > > > +\n" - "> > > > > > +\302\240\302\240\302\240\302\240\302\240\302\240\302\240asm volatile(\"1: wrussq %1, (%0)\\n\"\n" - "> > > > > > +\302\240\302\240\302\240\302\240\302\240\302\240\302\240\302\240\302\240\302\240\302\240\302\240\302\240\302\240\302\240\302\240\302\240\302\240\302\240\302\240\"2:\\n\"\n" - "> > > > > > +\302\240\302\240\302\240\302\240\302\240\302\240\302\240\302\240\302\240\302\240\302\240\302\240\302\240\302\240\302\240\302\240\302\240\302\240\302\240\302\240_ASM_EXTABLE_HANDLE(1b, 2b,\n" + "> > > > > > +A A A A A A A asm volatile(\"1: wrussq %1, (%0)\\n\"\n" + "> > > > > > +A A A A A A A A A A A A A A A A A A A A \"2:\\n\"\n" + "> > > > > > +A A A A A A A A A A A A A A A A A A A A _ASM_EXTABLE_HANDLE(1b, 2b,\n" "> > > > > > ex_handler_wruss)\n" - "> > > > > > +\302\240\302\240\302\240\302\240\302\240\302\240\302\240\302\240\302\240\302\240\302\240\302\240\302\240\302\240\302\240\302\240\302\240\302\240\302\240\302\240:\n" - "> > > > > > +\302\240\302\240\302\240\302\240\302\240\302\240\302\240\302\240\302\240\302\240\302\240\302\240\302\240\302\240\302\240\302\240\302\240\302\240\302\240\302\240: \"r\" (addr), \"r\" (val));\n" + "> > > > > > +A A A A A A A A A A A A A A A A A A A A :\n" + "> > > > > > +A A A A A A A A A A A A A A A A A A A A : \"r\" (addr), \"r\" (val));\n" "> > > > > > +\n" - "> > > > > > +\302\240\302\240\302\240\302\240\302\240\302\240\302\240return err;\n" + "> > > > > > +A A A A A A A return err;\n" "> > > > > > +}\n" "> > > > > What's up with \"err\"? You set it to zero, and then you return\n" "> > > > > it,\n" @@ -90,12 +95,12 @@ "> > > > > > \n" "> > > > > > +__visible bool ex_handler_wruss(const struct\n" "> > > > > > exception_table_entry *fixup,\n" - "> > > > > > +\302\240\302\240\302\240\302\240\302\240\302\240\302\240\302\240\302\240\302\240\302\240\302\240\302\240\302\240\302\240\302\240\302\240\302\240\302\240\302\240\302\240\302\240\302\240\302\240\302\240\302\240\302\240\302\240\302\240\302\240\302\240struct pt_regs *regs, int\n" + "> > > > > > +A A A A A A A A A A A A A A A A A A A A A A A A A A A A A A A struct pt_regs *regs, int\n" "> > > > > > trapnr)\n" "> > > > > > +{\n" - "> > > > > > +\302\240\302\240\302\240\302\240\302\240\302\240\302\240regs->ip = ex_fixup_addr(fixup);\n" - "> > > > > > +\302\240\302\240\302\240\302\240\302\240\302\240\302\240regs->ax = -1;\n" - "> > > > > > +\302\240\302\240\302\240\302\240\302\240\302\240\302\240return true;\n" + "> > > > > > +A A A A A A A regs->ip = ex_fixup_addr(fixup);\n" + "> > > > > > +A A A A A A A regs->ax = -1;\n" + "> > > > > > +A A A A A A A return true;\n" "> > > > > > +}\n" "> > > > > And here you just write into regs->ax, but your \"asm volatile\"\n" "> > > > > doesn't\n" @@ -103,32 +108,32 @@ "> > > > > \n" "> > > > > I think you probably want to add something like an explicit\n" "> > > > > `\"+&a\"(err)` output to the asm statements.\n" - "> > > > We require asm goto support these days.\302\240\302\240How about using\n" - "> > > > that?\302\240\302\240You\n" + "> > > > We require asm goto support these days.A A How about using\n" + "> > > > that?A A You\n" "> > > > won't even need a special exception handler.\n" - "> > Maybe something like this?\302\240\302\240It looks simple now.\n" + "> > Maybe something like this?A A It looks simple now.\n" "> > \n" "> > static inline int write_user_shstk_64(unsigned long addr, unsigned\n" "> > long val)\n" "> > {\n" - "> > \302\240\302\240\302\240\302\240\302\240\302\240\302\240\302\240asm_volatile_goto(\"wrussq %1, (%0)\\n\"\n" - "> > \302\240\302\240\302\240\302\240\302\240\302\240\302\240\302\240\302\240\302\240\302\240\302\240\302\240\302\240\302\240\302\240\302\240\302\240\302\240\302\240\302\240\"jmp %l[ok]\\n\"\n" - "> > \302\240\302\240\302\240\302\240\302\240\302\240\302\240\302\240\302\240\302\240\302\240\302\240\302\240\302\240\302\240\302\240\302\240\302\240\302\240\302\240\302\240\".section .fixup,\\\"ax\\\"n\"\n" - "> > \302\240\302\240\302\240\302\240\302\240\302\240\302\240\302\240\302\240\302\240\302\240\302\240\302\240\302\240\302\240\302\240\302\240\302\240\302\240\302\240\302\240\"jmp %l[fail]\\n\"\n" - "> > \302\240\302\240\302\240\302\240\302\240\302\240\302\240\302\240\302\240\302\240\302\240\302\240\302\240\302\240\302\240\302\240\302\240\302\240\302\240\302\240\302\240\".previous\\n\"\n" - "> > \302\240\302\240\302\240\302\240\302\240\302\240\302\240\302\240\302\240\302\240\302\240\302\240\302\240\302\240\302\240\302\240\302\240\302\240\302\240\302\240\302\240:: \"r\" (addr), \"r\" (val)\n" - "> > \302\240\302\240\302\240\302\240\302\240\302\240\302\240\302\240\302\240\302\240\302\240\302\240\302\240\302\240\302\240\302\240\302\240\302\240\302\240\302\240\302\240:: ok, fail);\n" + "> > A A A A A A A A asm_volatile_goto(\"wrussq %1, (%0)\\n\"\n" + "> > A A A A A A A A A A A A A A A A A A A A A \"jmp %l[ok]\\n\"\n" + "> > A A A A A A A A A A A A A A A A A A A A A \".section .fixup,\\\"ax\\\"n\"\n" + "> > A A A A A A A A A A A A A A A A A A A A A \"jmp %l[fail]\\n\"\n" + "> > A A A A A A A A A A A A A A A A A A A A A \".previous\\n\"\n" + "> > A A A A A A A A A A A A A A A A A A A A A :: \"r\" (addr), \"r\" (val)\n" + "> > A A A A A A A A A A A A A A A A A A A A A :: ok, fail);\n" "> > ok:\n" - "> > \302\240\302\240\302\240\302\240\302\240\302\240\302\240\302\240return 0;\n" + "> > A A A A A A A A return 0;\n" "> > fail:\n" - "> > \302\240\302\240\302\240\302\240\302\240\302\240\302\240\302\240return -1;\n" + "> > A A A A A A A A return -1;\n" "> > }\n" "> > \n" "> I think you can get rid of 'jmp %l[ok]' and the ok label and just fall\n" - "> through.\302\240\302\240And you don't need an explicit jmp to fail -- just set the\n" + "> through.A A And you don't need an explicit jmp to fail -- just set the\n" "> _EX_HANDLER entry to land on the fail label.\n" "\n" - "Thanks! \302\240This now looks simple and much better.\n" + "Thanks! A This now looks simple and much better.\n" "\n" "Yu-cheng\n" "\n" @@ -137,12 +142,12 @@ "+static inline int write_user_shstk_64(unsigned long addr, unsigned long val)\n" "+{\n" "+\tasm_volatile_goto(\"1: wrussq %1, (%0)\\n\"\n" - "+\t\t\t\302\240\302\240_ASM_EXTABLE(1b, %l[fail])\n" - "+\t\t\t\302\240\302\240:: \"r\" (addr), \"r\" (val)\n" - "+\t\t\t\302\240\302\240:: fail);\n" + "+\t\t\tA A _ASM_EXTABLE(1b, %l[fail])\n" + "+\t\t\tA A :: \"r\" (addr), \"r\" (val)\n" + "+\t\t\tA A :: fail);\n" "+\treturn 0;\n" "+fail:\n" "+\treturn -1;\n" +} -f1c329bc5d3b123c96aa462543811f4f526b31711be4e2c610ea41315a116a80 +8f144a5b71a76b791b9206f51e6935e3ca770a912b2ac3cc5b18b4dcb94a3941
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.