This is the mail archive of the
libc-alpha@sourceware.org
mailing list for the glibc project.
Re: [PATCH] fopencookie: Mangle function pointers stored on the heap [BZ #20222]
- From: Carlos O'Donell <carlos at redhat dot com>
- To: Florian Weimer <fweimer at redhat dot com>, libc-alpha at sourceware dot org
- Date: Fri, 10 Jun 2016 11:55:24 -0400
- Subject: Re: [PATCH] fopencookie: Mangle function pointers stored on the heap [BZ #20222]
- Authentication-results: sourceware.org; auth=none
- References: <20160610104919 dot 479B340176C1C at oldenburg dot str dot redhat dot com>
On 06/10/2016 06:49 AM, Florian Weimer wrote:
> 2016-06-10 Florian Weimer <fweimer@redhat.com>
>
> [BZ #20222]
> * libio/iofopncook.c (_IO_cookie_read): Demangle callback pointer.
> (_IO_cookie_write): Likewise.
> (_IO_cookie_seek): Likewise.
> (_IO_cookie_close): Likewise.
> (_IO_old_cookie_seek): Likewise.
> (set_callbacks): New function.
> (_IO_cookie_init): Call set_callbacks to copy callbacks.
A couple of questions...
> diff --git a/libio/iofopncook.c b/libio/iofopncook.c
> index 9eda7c1..487297c 100644
> --- a/libio/iofopncook.c
> +++ b/libio/iofopncook.c
> @@ -43,25 +43,29 @@ static _IO_ssize_t
> _IO_cookie_read (_IO_FILE *fp, void *buf, _IO_ssize_t size)
> {
> struct _IO_cookie_file *cfile = (struct _IO_cookie_file *) fp;
> + cookie_read_function_t *read_cb = cfile->__io_functions.read;
> + PTR_DEMANGLE (read_cb);
>
> - if (cfile->__io_functions.read == NULL)
> + if (read_cb == NULL)
> return -1;
>
> - return cfile->__io_functions.read (cfile->__cookie, buf, size);
> + return read_cb (cfile->__cookie, buf, size);
> }
>
> static _IO_ssize_t
> _IO_cookie_write (_IO_FILE *fp, const void *buf, _IO_ssize_t size)
> {
> struct _IO_cookie_file *cfile = (struct _IO_cookie_file *) fp;
> + cookie_write_function_t *write_cb = cfile->__io_functions.write;
> + PTR_DEMANGLE (write_cb);
>
> - if (cfile->__io_functions.write == NULL)
> + if (write_cb == NULL)
> {
> fp->_flags |= _IO_ERR_SEEN;
> return 0;
> }
>
> - _IO_ssize_t n = cfile->__io_functions.write (cfile->__cookie, buf, size);
> + _IO_ssize_t n = write_cb (cfile->__cookie, buf, size);
> if (n < size)
> fp->_flags |= _IO_ERR_SEEN;
>
> @@ -72,9 +76,11 @@ static _IO_off64_t
> _IO_cookie_seek (_IO_FILE *fp, _IO_off64_t offset, int dir)
> {
> struct _IO_cookie_file *cfile = (struct _IO_cookie_file *) fp;
> + cookie_seek_function_t *seek_cb = cfile->__io_functions.seek;
> + PTR_DEMANGLE (seek_cb);
>
> - return ((cfile->__io_functions.seek == NULL
> - || (cfile->__io_functions.seek (cfile->__cookie, &offset, dir)
> + return ((seek_cb == NULL
> + || (seek_cb (cfile->__cookie, &offset, dir)
> == -1)
> || offset == (_IO_off64_t) -1)
> ? _IO_pos_BAD : offset);
> @@ -84,11 +90,13 @@ static int
> _IO_cookie_close (_IO_FILE *fp)
> {
> struct _IO_cookie_file *cfile = (struct _IO_cookie_file *) fp;
> + cookie_close_function_t *close_cb = cfile->__io_functions.close;
> + PTR_DEMANGLE (close_cb);
>
> - if (cfile->__io_functions.close == NULL)
> + if (close_cb == NULL)
> return 0;
>
> - return cfile->__io_functions.close (cfile->__cookie);
> + return close_cb (cfile->__cookie);
> }
>
>
> @@ -126,6 +134,19 @@ static const struct _IO_jump_t _IO_cookie_jumps = {
> };
>
>
> +/* Copy the callbacks from SOURCE to *TARGET, with pointer
> + mangling. */
> +static void
> +set_callbacks (_IO_cookie_io_functions_t *target,
> + _IO_cookie_io_functions_t source)
> +{
> + PTR_MANGLE (source.read);
> + PTR_MANGLE (source.write);
> + PTR_MANGLE (source.seek);
> + PTR_MANGLE (source.close);
Isn't this modifying the user's copy of the callbacks
passed in during the call to fopencookie?
If it is, then that might be unexpected. I expect that we
need to copy the function pointers and then encrypt them.
Leaving the users copy untouched.
In fact I'd go as far to say that stdio-common/tst-cookie.c
should be modified to verify that 'cookie_io_functions_t fcts'
is unmodified after the call to fopencookie.
> + *target = source;
> +}
> +
> void
> _IO_cookie_init (struct _IO_cookie_file *cfile, int read_write,
> void *cookie, _IO_cookie_io_functions_t io_functions)
> @@ -134,7 +155,7 @@ _IO_cookie_init (struct _IO_cookie_file *cfile, int read_write,
> _IO_JUMPS (&cfile->__fp) = &_IO_cookie_jumps;
>
> cfile->__cookie = cookie;
> - cfile->__io_functions = io_functions;
> + set_callbacks (&cfile->__io_functions, io_functions);
>
> _IO_file_init (&cfile->__fp);
>
> @@ -205,14 +226,14 @@ attribute_compat_text_section
> _IO_old_cookie_seek (_IO_FILE *fp, _IO_off64_t offset, int dir)
> {
> struct _IO_cookie_file *cfile = (struct _IO_cookie_file *) fp;
> - int (*seek) (_IO_FILE *, _IO_off_t, int);
> - int ret;
> + int (*seek_cb) (_IO_FILE *, _IO_off_t, int)
> + = (int (*)(_IO_FILE *, _IO_off_t, int)) cfile->__io_functions.seek;;
> + PTR_DEMANGLE (seek_cb);
>
> - seek = (int (*)(_IO_FILE *, _IO_off_t, int)) cfile->__io_functions.seek;
> - if (seek == NULL)
> + if (seek_cb == NULL)
> return _IO_pos_BAD;
>
> - ret = seek (cfile->__cookie, offset, dir);
> + int ret = seek_cb (cfile->__cookie, offset, dir);
>
> return (ret == -1) ? _IO_pos_BAD : ret;
> }
>
--
Cheers,
Carlos.