All of lore.kernel.org
 help / color / mirror / Atom feed
From: Al Viro <viro@ZenIV.linux.org.uk>
To: Linus Torvalds <torvalds@linux-foundation.org>
Cc: linux-kernel@vger.kernel.org, "H. Peter Anvin" <hpa@zytor.com>,
	Ralf Baechle <ralf@linux-mips.org>
Subject: Re: [PATCH] broken TASK_SIZE for ia32_aout
Date: Sun, 6 May 2012 18:54:51 +0100	[thread overview]
Message-ID: <20120506175451.GU6871@ZenIV.linux.org.uk> (raw)
In-Reply-To: <CA+55aFzj3Ocjpnw3WdmKjp3oELwmXt=Zs2=fZuYy0TUyF4nXiA@mail.gmail.com>

On Sun, May 06, 2012 at 10:16:11AM -0700, Linus Torvalds wrote:
> On Sun, May 6, 2012 at 9:20 AM, Al Viro <viro@zeniv.linux.org.uk> wrote:
> > Setting TIF_IA32 in load_aout_binary() used to be enough; these days
> > TASK_SIZE is controlled by TIF_ADDR32 and that one doesn't get set
> > there. ?Switch to use of set_personality_ia32()...
> 
> Applied. Just out of curiosity, how did you notice? Just looking at
> TIF_IA32 usage, or do you actually have some old app?

Putting together an idiot's guide to thread flags ;-)  Other very similar
bug, AFAICS, is in mips elf.h:
#define __SET_PERSONALITY32_N32()                                       \
        do {                                                            \
                set_thread_flag(TIF_32BIT_ADDR);                        \
                current->thread.abi = &mips_abi_n32;                    \
        } while (0)
probably should clear_thread_flag(TIF_32BIT_REGS); as well.  Ralf?

Another catch on mips, and that one is downright funny:
#ifdef CONFIG_MIPS32_O32
#define TIF_32BIT TIF_32BIT_REGS
#elif defined(CONFIG_MIPS32_N32)
#define TIF_32BIT _TIF_32BIT_ADDR
#endif /* CONFIG_MIPS32_O32 */

Spot the breakage.  Note that TIF_32BIT is used only in is_compat_task()
there, so that went unnoticed for a long time.  BTW, I really wonder if
the logics here is correct, nevermind the obvious typo...  For kernels
with O32, but not N32 support we could bloody well use TIF_32BIT_ADDR -
O32 sets both flags.  And for kernels that support both (O32 and N32 are
not mutually exclusive), we probably want both of o32 and n32 processes
match is_compat_task().

Note that we have different semantics for is_compat_task() on x86 and
everywhere (AFAICS) else: "is this a 32bit syscall" vs. "is this a biarch
syscall with 32bit pointers, etc.".  On architectures where 32bit syscall
can't be issued by 64bit task and vice versa there's no difference, but for
e.g. sparc there definitely is one.  is_compat_task() there goes by what
the task is, not what kind of syscall is it trying to make.  It mostly
doesn't matter (is_compat_task() has very few users), but I suspect that
for e.g. ext4 is_32bit_api() it does matter and is currently broken...

What kind of semantics do we want?  "Thread property" one, set when we
set personality on execve(), or "syscall property", like e.g. x86 TIF_IRET
and TS_COMPAT?

  parent reply	other threads:[~2012-05-06 17:54 UTC|newest]

Thread overview: 20+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2012-05-06 16:20 [PATCH] broken TASK_SIZE for ia32_aout Al Viro
2012-05-06 17:16 ` Linus Torvalds
2012-05-06 17:37   ` H. Peter Anvin
2012-05-06 17:54   ` Al Viro [this message]
2012-05-06 17:56     ` Al Viro
2012-05-06 17:58     ` H. Peter Anvin
2012-05-06 18:46       ` Al Viro
2012-05-06 18:48         ` H. Peter Anvin
2012-05-06 19:58           ` Al Viro
2012-05-06 20:32           ` David Miller
2012-05-06 20:51             ` H. Peter Anvin
2012-05-06 23:40               ` David Miller
2012-05-06 23:48                 ` H. Peter Anvin
2012-05-07  2:56                   ` David Miller
2012-05-07  3:05                     ` H. Peter Anvin
2012-05-06 23:32             ` Al Viro
2012-05-06 23:38               ` David Miller
2012-05-07  0:13                 ` Al Viro
2012-05-07  0:24     ` Al Viro
2012-05-06 17:23 ` [tip:x86/urgent] x86, compat: Correct broken TASK_SIZE for ia32 a. out binaries tip-bot for Al Viro

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=20120506175451.GU6871@ZenIV.linux.org.uk \
    --to=viro@zeniv.linux.org.uk \
    --cc=hpa@zytor.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=ralf@linux-mips.org \
    --cc=torvalds@linux-foundation.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 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.