[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <20170628151328.GD1627@brightrain.aerifal.cx>
Date: Wed, 28 Jun 2017 11:13:28 -0400
From: Rich Felker <dalias@...c.org>
To: musl@...ts.openwall.com
Subject: Re: [PATCH] fix undefined behavior in ptrace
On Wed, Jun 28, 2017 at 04:25:13PM +0300, Alexander Monakov wrote:
> ---
> src/linux/ptrace.c | 5 ++++-
> 1 file changed, 4 insertions(+), 1 deletion(-)
>
> diff --git a/src/linux/ptrace.c b/src/linux/ptrace.c
> index 83b8022b..ab7fcda3 100644
> --- a/src/linux/ptrace.c
> +++ b/src/linux/ptrace.c
> @@ -7,14 +7,17 @@ long ptrace(int req, ...)
> {
> va_list ap;
> pid_t pid;
> - void *addr, *data, *addr2;
> + void *addr, *data, *addr2 = 0;
> long ret, result;
>
> va_start(ap, req);
> pid = va_arg(ap, pid_t);
> addr = va_arg(ap, void *);
> data = va_arg(ap, void *);
> + /* PTRACE_{READ,WRITE}{DATA,TEXT} are specific to SPARC. */
> +#ifdef PTRACE_READTEXT
> addr2 = va_arg(ap, void *);
> +#endif
I think there's still UB here, reading more args than were passed.
These calls to va_arg should probably be dependent on the particular
req; I don't see any reason for it to be compile-time dependent on the
presence of one particular req value.
Otherwise yes it's an improvement.
Rich
Powered by blists - more mailing lists
Confused about mailing lists and their use?
Read about mailing lists on Wikipedia
and check out these
guidelines on proper formatting of your messages.
