All of lore.kernel.org
 help / color / mirror / Atom feed
From: "Nikita V. Youshchenko" <yoush-/llMDZXAvAOHXe+LvDLADg@public.gmane.org>
To: Matt Helsley <matthltc-r/Jw6+rmf7HQT0dZR+AlfA@public.gmane.org>
Cc: containers-cunTk1MwBs9QetFLy7KEm3xJsTq8ys+cHZ5vskTnxNA@public.gmane.org,
	Leonid Snegirev <leo-n4oKp6kCDthKyFCjRbgQbg@public.gmane.org>
Subject: Re: [PATCH] c/r: restore tty pgrp properly
Date: Fri, 9 Apr 2010 11:47:00 +0400	[thread overview]
Message-ID: <201004091147.01571@zigzag.lvk.cs.msu.su> (raw)
In-Reply-To: <20100408210008.GF3345-52DBMbEzqgQ/wnmkkaCWp/UQ3DHhIser@public.gmane.org>

> Do you have a testcase where restart doesn't work because of problems
> with tty->pgrp restore? Or was this just something that caught your eye
> during review?

Yes, we faced this while trying to restore an Xvnc session (bash running a 
script + Xvnc + twm + xterm + bash). It was failing with -ESRCH at this 
particular point, and we have been trying to solve it.

> Userspace can change h->tty_pgrp_owner to any value (bug, cosmic ray,
> exploit attempt) before passing the checkpoint image to restart. We
> don't want malicious programs to be able to cause an Oops or worse so we
> need to make sure the index is less than the number of entries in the
> pids_arr. Since the value was a prgrp id prior to this patch I doubt
> that check already exists.

Understood.

It should be not

if (h->tty_pgrp_owner != -1), but

but

if ( h->tty_pgrp_owner >=0 && h->tty_pgrp_owner < ctx->nr_tasks)

> However, I suspect this is already covered in userspace
> by rewriting the pids array prior to calling sys_restart(). See
> ckpt_init_tree() in user-cr's restart.c and let me know if you agree
> and/or see any problems with that approach.

This may be indeed better, but it requires userspace to have knowledge 
about entire checkpoint structure, with (as far as I understand) is not 
the case currently, and may be hard to maintain while checkpoint format 
envolves in future.

Nikita

      parent reply	other threads:[~2010-04-09  7:47 UTC|newest]

Thread overview: 4+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2010-04-08 15:50 [PATCH] c/r: restore tty pgrp properly Leonid Snegirev
     [not found] ` <4BBDFB60.2080505-n4oKp6kCDthKyFCjRbgQbg@public.gmane.org>
2010-04-08 21:00   ` Matt Helsley
     [not found]     ` <20100408210008.GF3345-52DBMbEzqgQ/wnmkkaCWp/UQ3DHhIser@public.gmane.org>
2010-04-09  3:23       ` Oren Laadan
2010-04-09  7:47       ` Nikita V. Youshchenko [this message]

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=201004091147.01571@zigzag.lvk.cs.msu.su \
    --to=yoush-/llmdzxavaohxe+lvdladg@public.gmane.org \
    --cc=containers-cunTk1MwBs9QetFLy7KEm3xJsTq8ys+cHZ5vskTnxNA@public.gmane.org \
    --cc=leo-n4oKp6kCDthKyFCjRbgQbg@public.gmane.org \
    --cc=matthltc-r/Jw6+rmf7HQT0dZR+AlfA@public.gmane.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.