linux-kernel - Re: [patch v3 2/2] ptrace: Add support for generic PTRACE_GETREGSET/PTRACE

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-Id: <20100212035606.119B3C821@magilla.sf.frob.com>
Date:	Thu, 11 Feb 2010 19:56:06 -0800 (PST)
From:	Roland McGrath <roland@...hat.com>
To:	Suresh Siddha <suresh.b.siddha@...el.com>
Cc:	Oleg Nesterov <oleg@...hat.com>, "H. Peter Anvin" <hpa@...or.com>,
	Ingo Molnar <mingo@...e.hu>,
	Thomas Gleixner <tglx@...utronix.de>,
	LKML <linux-kernel@...r.kernel.org>, hjl.tools@...il.com,
	peter.lachner@...el.com
Subject: Re: [patch v3 2/2] ptrace: Add support for generic PTRACE_GETREGSET/PTRACE_SETREGSET

Note that this patch and the xstate user_regset patch are entirely independent.
They can me merged in any order or one without the other.

> +/*
> + * Generic ptrace interface that exports the architecture specific regsets
> + * using the corresponding NT_* types (which are also used in the core dump).

There is a special case here, which I think already works as we intend it
to, but which should be clarified in the comment about this API.  The
NT_PRSTATUS note type in a core dump contains a full 'struct elf_prstatus'.
But the user_regset for NT_PRSTATUS contains just the elf_gregset_t that
is the pr_reg field of 'struct elf_prstatus'.

For all the other user_regset flavors, the user_regset layout and the ELF
core dump note payload are exactly the same layout, as your comment implies.

> +static int ptrace_regset(struct task_struct *task, int req, unsigned int type,
> +			 struct iovec *kiov)
> +{
> +	const struct user_regset_view *view = task_user_regset_view(task);
> +	const struct user_regset *regset = find_regset(view, type);
> +	int regset_no;
> +
> +	if (!regset || (kiov->iov_len % regset->size) != 0)
> +		return -EIO;

My inclination would be to diagnose these more specifically.  For a bad
size, give -EINVAL.  For an unknown regset type, give maybe -EINVAL or
maybe -ENODEV.  (-ENODEV is what you get for a known NT_* type that has a
user_regset implemented in the kernel, but that the particular hardware
we're running on doesn't support.  So perhaps you don't want to overload
that for a wholly unrecognized NT_* type.)

Otherwise, looks good to me.  ACK contingent on Oleg's ACK.

Thanks,
Roland
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/