On 05/01/2014 07:43 PM, Pedro Alves wrote:
> 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};
:) Thats not explicit enough.
>
>> @@ -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.
>
Yeah, sounds better. I may add "If the length is zero, the kernel will neither
read
from or write into the buffer"
> 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.
No, it does not have to. From the code snippet below (ptrace_regset function)
the buffer length has to be multiple of regset->size for the given NT_XXX_TYPE
upto the max regset size for the user to see any valid data. The problem what I
faced was when you use any iovec structure with the length parameter
uninitialized,
the kernel simply ignores and does not return anything.
if (!regset || (kiov->iov_len % regset->size) != 0)
return -EINVAL;
>
>> 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.
No, I am not saying that. The kernel takes care of that situation by capping
the length to regset size of the NT_XXX_TYPE.
kiov->iov_len = min(kiov->iov_len,
(__kernel_size_t) (regset->n * regset->size));
Shall I resend the patch with the your proposed changes and your
"Signed-off-by" and
moving myself as "Reported-by" ?
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
