[Devel] Re: [PATCH 3/4] c/r: [signal] pending signals (private, shared)
Oren Laadan
orenl at librato.com
Mon Aug 17 13:14:29 PDT 2009
Dan Smith wrote:
> OL> +static int load_siginfo(siginfo_t *info, struct ckpt_hdr_siginfo *si)
> OL> +{
> OL> + if (!valid_signal(si->signo))
> OL> + return -EINVAL;
> OL> +
> OL> + info->si_signo = si->signo;
> OL> + info->si_errno = si->_errno;
> OL> + info->si_code = si->code;
> OL> +
> OL> + /* TODO: validate remaining signal fields */
> OL> +
> OL> + switch(info->si_code & __SI_MASK) {
> OL> + case __SI_TIMER:
> OL> + info->si_tid = si->pid;
> OL> + info->si_overrun = si->uid;
> OL> + info->si_int = si->sigval_int;
> OL> + info->si_sys_private = si->utime;
> OL> + break;
> OL> + case __SI_POLL:
> OL> + info->si_band = si->pid;
> OL> + info->si_fd = si->sigval_int;
> OL> + break;
> OL> + case __SI_FAULT:
> OL> + info->si_addr = (void __user *) (unsigned long) si->sigval_ptr;
> OL> +#ifdef __ARCH_SI_TRAPNO
> OL> + info->si_trapno = si->sigval_int;
> OL> +#endif
> OL> + break;
> OL> + case __SI_CHLD:
> OL> + info->si_pid = si->pid;
> OL> + info->si_uid = si->uid;
> OL> + info->si_status = si->sigval_int;
> OL> + info->si_stime = si->stime;
> OL> + info->si_utime = si->utime;
> OL> + break;
> OL> + case __SI_KILL:
> OL> + case __SI_RT:
> OL> + case __SI_MESGQ:
> OL> + info->si_pid = si->pid;
> OL> + info->si_uid = si->uid;
> OL> + info->si_ptr = (void __user *) (unsigned long) si->sigval_ptr;
> OL> + break;
> OL> + default:
> OL> + return -EINVAL;
> OL> + }
> OL> +
> OL> + return 0;
> OL> +}
> OL> +
>
> This seems like a perfect place to use the CKPT_COPY() macros, if
> we're going to have them. This and the save equivalent could be
> almost identical.
Hehe... "if" ?!
Anyway, I'll see what I can do.
> OL> +static int restore_sigpending(struct ckpt_ctx *ctx, struct sigpending *pending)
> OL> +{
> OL> + struct ckpt_hdr_sigpending *h;
> OL> + struct ckpt_hdr_siginfo *si;
> OL> + struct sigqueue *q;
> OL> + int ret = 0;
> OL> +
> OL> + h = ckpt_read_buf_type(ctx, 0, CKPT_HDR_SIGPENDING);
> OL> + if (IS_ERR(h))
> OL> + return PTR_ERR(h);
> OL> +
> OL> + INIT_LIST_HEAD(&pending->list);
> OL> + load_sigset(&pending->signal, &h->signal);
> OL> +
> OL> + si = h->siginfo;
> OL> + while (h->nr_pending--) {
> OL> + q = sigqueue_alloc();
> OL> + if (!q) {
> OL> + ret = -ENOMEM;
> OL> + break;
> OL> + }
> OL> +
> OL> + ret = load_siginfo(&q->info, si++);
>
> I think there should be a sanity check here, no? The checkpoint
> stream claims h->nr_pending structures in h->siginfo, but we can't
> trust that we're not going to march through memory to a segv unless we
> check that the header length matches your calculation in
> checkpoint_sigpending().
>
I think you are right :p
Thanks,
Oren.
_______________________________________________
Containers mailing list
Containers at lists.linux-foundation.org
https://lists.linux-foundation.org/mailman/listinfo/containers
More information about the Devel
mailing list