public inbox for systemtap@sourceware.org
 help / color / mirror / Atom feed
From: "dsmith at redhat dot com" <sourceware-bugzilla@sourceware.org>
To: systemtap@sourceware.org
Subject: [Bug tapsets/18597] long_arg() doesn't correctly handle negative values in 32-on-64 environment
Date: Tue, 30 Jun 2015 15:56:00 -0000	[thread overview]
Message-ID: <bug-18597-6586-Y7vBkhWyHo@http.sourceware.org/bugzilla/> (raw)
In-Reply-To: <bug-18597-6586@http.sourceware.org/bugzilla/>

https://sourceware.org/bugzilla/show_bug.cgi?id=18597

--- Comment #5 from David Smith <dsmith at redhat dot com> ---
(In reply to David Smith from comment #4)
> If this code wasn't needed before, why is it needed now?

I've figured this out. This code is needed because stp_arg() (which long_arg()
calls) had a bug. The bug was that in some circumstances, longlong_arg() would
return a 64-bit value (from 1 register) when called on a 32-bit process.
longlong_arg()/stp_arg() was designed to do the following:

If we're in a 32-bit compat process, then you can't put a 64-bit value in 1
register. Instead, you have to break the 64-bit value up and put it in 2
registers. This is why longlong_arg() had code in it like this:

-       if (probing_32bit_app()) {
-               lowbits = _stp_arg(argnum, 0, 1)
-               highbits = _stp_arg(argnum+1, 0, 1)
-               return ((highbits << 32) | lowbits)

However, this code doesn't really work correctly on a 64-bit OS. Let me give an
example. Here's pread() and the x86_64 compat function for pread(). The 'pos'
argument (or the combination of 'poslo' and 'poshi' arguments) is a 64-bit
value.

====
ssize_t sys_pread64(unsigned int fd, char __user *buf, size_t count, loff_t
pos)
{
  //...
}

asmlinkage long sys32_pread(unsigned int fd, char __user *ubuf, u32 count,
                            u32 poslo, u32 poshi)
{
        return sys_pread64(fd, ubuf, count,
                         ((loff_t)AA(poshi) << 32) | AA(poslo));
}
====

If we're probing a compat function, we'd never call longlong_arg(), we'd do the
same thing that the wrapper function is doing - call ulong_arg() on arguments 4
and 5 and slam the values together. Since probing_32bit_app() never returned
true, the supposed 32-bit code in longlong_arg() was never getting called.

Your patch has to add lots of "compat" probes because as a side effect of
fixing the negative value problem it "broke" longlong_arg() returning a 64-bit
value from 1 register even when in a 32-bit process (which shouldn't have
worked in the first place).

I've been struggling with the right way to fix this. I went down the same path
you tried, and ended up about in the same place. Then I decided to try a
different path.

What if we decided that if you call longlong_arg() on a 64-bit OS on a 32-bit
process you really *want* a 64-bit value from 1 register? In some ways this
makes sense and it matches our old behavior. My theory here is that you know
what you are doing and if you call longlong_arg() on a 32-bit process you must
be in the true 64-bit function at this point.

I'm testing this now and it works well on everything but ppc64. I'm unsure of
what is going on there.

-- 
You are receiving this mail because:
You are the assignee for the bug.

  parent reply	other threads:[~2015-06-30 15:56 UTC|newest]

Thread overview: 22+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2015-06-25 11:00 [Bug tapsets/18597] New: " mcermak at redhat dot com
2015-06-25 11:03 ` [Bug tapsets/18597] " mcermak at redhat dot com
2015-06-25 11:04 ` mcermak at redhat dot com
2015-06-25 15:03 ` mcermak at redhat dot com
2015-06-26 12:13 ` dsmith at redhat dot com
2015-06-26 12:20 ` dsmith at redhat dot com
2015-06-30 15:56 ` dsmith at redhat dot com [this message]
2015-06-30 15:59 ` dsmith at redhat dot com
2015-06-30 17:21 ` jistone at redhat dot com
2015-06-30 17:36 ` dsmith at redhat dot com
2015-06-30 20:21 ` dsmith at redhat dot com
2015-06-30 20:22 ` dsmith at redhat dot com
2015-06-30 20:32 ` jistone at redhat dot com
2015-06-30 20:46 ` dsmith at redhat dot com
2015-07-01 15:21 ` mcermak at redhat dot com
2015-07-01 16:14 ` dsmith at redhat dot com
2015-07-01 17:10 ` mcermak at redhat dot com
2015-07-01 17:29 ` dsmith at redhat dot com
2015-07-03 14:18 ` mcermak at redhat dot com
2015-07-06 13:45 ` dsmith at redhat dot com
2015-07-07  7:02 ` mcermak at redhat dot com
2015-07-08  6:09 ` mcermak at redhat dot com

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=bug-18597-6586-Y7vBkhWyHo@http.sourceware.org/bugzilla/ \
    --to=sourceware-bugzilla@sourceware.org \
    --cc=systemtap@sourceware.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).