[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [Xen-devel] [PATCH] x86: slightly improve stack trace on debug builds


  • To: Jan Beulich <JBeulich@xxxxxxxx>
  • From: Keir Fraser <keir@xxxxxxx>
  • Date: Tue, 25 Sep 2012 18:06:17 +0100
  • Cc: xen-devel <xen-devel@xxxxxxxxxxxxx>
  • Delivery-date: Tue, 25 Sep 2012 17:06:53 +0000
  • List-id: Xen developer discussion <xen-devel.lists.xen.org>
  • Thread-index: Ac2bQBNxsgPFijZEwUensymehOG/rQ==
  • Thread-topic: [Xen-devel] [PATCH] x86: slightly improve stack trace on debug builds

On 25/09/2012 17:20, "Jan Beulich" <JBeulich@xxxxxxxx> wrote:

>> /*
>>  * If RIP is not valid hypervisor code then someone may have called into
>>  * oblivion. Peek to see if they left a return address at top of stack.
>>  */
>> addr = (!is_current_kernel_text(regs->eip) &&
>>         (ESP_BEFORE_EXCEPTION(regs) >= low) &&
>>         (ESP_BEFORE_EXCEPTION(regs) < high) &&
>>         is_current_kernel_text(*ESP_BEFORE_EXCEPTION(regs)))
>>        ? *ESP_BEFORE_EXCEPTION(regs) : regs->eip;
> 
> Checking against "low" is pointless, as that one is guaranteed
> lower than the stack pointer (unless the stack pointer was within
> 16 bytes from NULL). Checking against "high" is almost pointless
> too, as there's only a very small range where %rsp could have
> been to make that check fail (plus the 2 slots offset wouldn't be
> right in this special case, i.e. we could get a false negative here),
> and it wouldn't help preventing eventual #PF on the deref (since
> "high" and the possible window above are on the same page).

Ah of course low and high are derived from ESP_BEFORE_EXCEPTION... Okay,
then my extra checks are quite pointless as you say. Please leave them out
when you re-spin your patch.

 Thanks,
 Keir



_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
http://lists.xen.org/xen-devel


 


Rackspace

Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.