On Mon, May 1, 2017 at 8:54 AM, Josh Poimboeuf <jpoim...@redhat.com> wrote: > On Thu, Apr 27, 2017 at 01:22:05PM -0700, Kees Cook wrote: >> On Wed, Apr 26, 2017 at 6:31 PM, kbuild test robot <l...@intel.com> wrote: >> > Hi Kees, >> > >> > [auto build test WARNING on next-20170424] >> > [cannot apply to tip/x86/core linus/master linux/master v4.9-rc8 v4.9-rc7 >> > v4.9-rc6 v4.11-rc8] >> > [if your patch is applied to the wrong git tree, please drop us a note to >> > help improve the system] >> > >> > url: >> > https://github.com/0day-ci/linux/commits/Kees-Cook/x86-refcount-Implement-fast-refcount-overflow/20170426-210530 >> > config: x86_64-allmodconfig (attached as .config) >> > compiler: gcc-6 (Debian 6.2.0-3) 6.2.0 20160901 >> > reproduce: >> > # save the attached .config to linux build tree >> > make ARCH=x86_64 >> > >> > All warnings (new ones prefixed by >>): >> > >> >>> drivers//scsi/scsi_scan.o: warning: objtool: >> >>> .text.refcount_overflow+0x5: special: can't find orig instruction >> >> Hi Josh, >> >> I'm seeing this error being generated on areas that are using a >> cross-section exception handler. I can't quite see why the .o checker >> is unhappy, so I figured I'd ask you first. :) >> >> The code is generated with calls to __REFCOUNT_CHECK() which is >> defined like this: >> >> +#define __REFCOUNT_EXCEPTION(size) \ >> + ".if "__stringify(size)" == 4\n\t" \ >> + ".pushsection .text.refcount_overflow\n" \ >> + ".elseif "__stringify(size)" == -4\n\t" \ >> + ".pushsection .text.refcount_underflow\n" \ >> + ".else\n" \ >> + ".error \"invalid size\"\n" \ >> + ".endif\n" \ >> + "111:\tlea %[counter],%%"_ASM_CX"\n\t" \ >> + "int $"__stringify(X86_REFCOUNT_VECTOR)"\n" \ >> + "222:\n\t" \ >> + ".popsection\n" \ >> + "333:\n" \ >> + _ASM_EXTABLE(222b, 333b) >> + >> +#define __REFCOUNT_CHECK(size) \ >> + "js 111f\n" \ >> + __REFCOUNT_EXCEPTION(size) >> + >> +#define __REFCOUNT_ERROR(size) \ >> + "jmp 111f\n" \ >> + __REFCOUNT_EXCEPTION(size) >> >> I assume it doesn't like seeing an exception split across .text and >> .text.refcount_overflow, but I haven't been able to figure out how >> that distinction would be made by the checker. :P > > This code uses the exception table a little differently than normal. > Usually it's used for catching page faults, where the exception table > points to the faulting instruction. > > But instead of a page fault, here it's doing a software interrupt. So > the __ex_table entry doesn't point to the 'int 0x81' instruction, it > points to the instruction immediately after it. In this case there > isn't actually an instruction there, which is why objtool is > complaining.
What would it take to adjust objtool for this case? > > Is it superfluous to use the exception table here, when a simple 'jmp > 333f' could be used instead after the 'int'? I thought the exception tables were needed to have the trap handler notice it correctly, and do the right thing as far as continuing execution. (This is currently written as a survivable condition: the kernel can keep running even though it will kill the userspace process.) > Also it looks like the handler sends a SIGKILL to the current task. I > wonder if something like BUG_ON() could be used instead of implementing > a custom error interrupt. It's a rate limited report, but it must always kill. BUG doesn't fit this usage case (I've got similar problems with other areas; my intention is go create something that is configurable WARN vs Oops, respects panic_on_oops, etc, but this doesn't exist yet). -Kees -- Kees Cook Pixel Security
