linux-kernel - Re: [PATCH] ptrace: Fix 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: <53625681.8010905@redhat.com>
Date:	Thu, 01 May 2014 15:13:21 +0100
From:	Pedro Alves <palves@...hat.com>
To:	Anshuman Khandual <khandual@...ux.vnet.ibm.com>
CC:	linux-kernel@...r.kernel.org, hpa@...or.com,
	suresh.b.siddha@...el.com
Subject: Re: [PATCH] ptrace: Fix PTRACE_GETREGSET/PTRACE_SETREGSET in code
 documentation

On 04/28/2014 12:00 PM, Anshuman Khandual wrote:
> The current documentation is bit misleading and does not explicitly
> specify that iov.len need to be initialized failing which kernel
> may just ignore the ptrace request and never read from/write into
> the user specified buffer. This patch fixes the documentation.

Well, it kind of does, here:

*      struct iovec iov = { buf, len};

> @@ -43,8 +43,12 @@
>   *
>   *	ret = ptrace(PTRACE_GETREGSET/PTRACE_SETREGSET, pid, NT_XXX_TYPE, &iov);
>   *
> - * On the successful completion, iov.len will be updated by the kernel,
> - * specifying how much the kernel has written/read to/from the user's iov.buf.
> + * A non-zero value upto the max size of data expected to be written/read by the
> + * kernel in response to any NT_XXX_TYPE request type must be assigned to iov.len
> + * before initiating the ptrace call. If iov.len is 0, then kernel will neither
> + * read from or write into the user buffer specified. On successful completion,
> + * iov.len will be updated by the kernel, specifying how much the kernel has
> + * written/read to/from the user's iov.buf.

I really appreciate that you're trying to make this clearer, but I
find the new sentence very hard to read/reason.  :-/

I suggest:

 * This interface usage is as follows:
- *      struct iovec iov = { buf, len};
+ *      struct iovec iov = { buf, len };
 *
 *      ret = ptrace(PTRACE_GETREGSET/PTRACE_SETREGSET, pid, NT_XXX_TYPE, &iov);
 *
- * On the successful completion, iov.len will be updated by the kernel,
- * specifying how much the kernel has written/read to/from the user's iov.buf.
+ * On entry, iov describes the buffer's address and length.  The buffer's
+ * length must be equal to or shorter than the size of the NT_XXX_TYPE regset.
+ * On successful completion, iov.len is updated by the kernel, specifying how
+ * much the kernel has written/read to/from the user's iov.buf.

I'm not sure I understood what you're saying correctly, though.  Specifically,
I don't know whether the buffer's length must really be shorter than the
size of the NT_XXX_TYPE regset.

> The current documentation is bit misleading and does not explicitly
> specify that iov.len need to be initialized failing which kernel
> may just ignore the ptrace request and never read from/write into
> the user specified buffer.

You're saying that if iov.len is larger than the NT_XXX_TYPE regset,
then the kernel returns _success_, but actually doesn't fill the
buffer?  That sounds like a bug to me.

-- 
Pedro Alves

--
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/