[Devel] [PATCH vz7] x86,ia32: Restore 32bit personality

Cyrill Gorcunov gorcunov at virtuozzo.com
Fri Nov 10 19:21:39 MSK 2017


On Fri, Nov 10, 2017 at 04:04:01PM +0000, Dmitry Safonov wrote:
> > +       if (regs->cs == __USER32_CS) {
> > +               /*
> > +                * It's close to set_personality_ia32
> > +                * but we don't want to change orig_ax.
> > +                */
> > +               set_thread_flag(TIF_ADDR32);
> > +               set_thread_flag(TIF_IA32);
> > +               clear_thread_flag(TIF_X32);
> > +               current->personality |= force_personality32;
> > +               current_thread_info()->status |= TS_COMPAT;
> > +               if (current->mm)
> > +                       current->mm->context.ia32_compat = TIF_IA32;
> > +       }
> 
> I see. The thing is that setting this thread-flag may be considered
> racy from the kernel side.

Well, to be fair indeed there is no lock and potential place for
a race, but since we're inside syscall handler I don't think
we hit any problem. I'll revisit locking, thanks! And note the
only purpose for this patch is to support criu since normal
programs don't usually do such things.

> So, the way mainstream accepted ia32 C/R is that this TIF_IA32
> will not be switched from userspace. Instead, this flag is considered
> for removing. And the task from kernel perspective shouldn't differ
> with exception for which syscall they are doing (ia32/x32/64-bit).
> While I've removed the major users (like signal ABI format delivery,
> ptrace and etc), there are a couple of minor users left like oprofile,
> uprobes - and the way they use this flag is not critical for their work.
> 
> So, it looks like this difference between vz and ms has slipped my
> mind. I don't mind your change, but have you tried something less
> intrusive? Something like the attached patch.

Yes, but I can't fetch the vanilla patches because they are a
way too big. While I like the idea of your patch we have to
do somthing with do_signal as well

		case -ERESTART_RESTARTBLOCK:
			regs->ax = NR_restart_syscall;
			regs->ip -= 2;
			break;

where

define NR_restart_syscall	\
	test_thread_flag(TIF_IA32) ? __NR_ia32_restart_syscall : __NR_restart_syscall

So that TIF_IA32 more intrusive than might be see in first glance.
Still thanks a lot! I'll take more precisely on Mon maybe extending
your patch will be less code. Thanks!


More information about the Devel mailing list