[PATCH libinput v2] Fix to avoid use of undefined pointer values.
Jasper St. Pierre
jstpierre at mecheye.net
Fri May 29 10:38:43 PDT 2015
I can't find any standard for asprintf outside of the glibc library,
which doesn't mention anything about undefined behavior. Am I missing
something?
http://www.gnu.org/software/libc/manual/html_node/Dynamic-Output.html
On Fri, May 29, 2015 at 10:29 AM, Jon A. Cruz <jonc at osg.samsung.com> wrote:
> Well, no.
>
> It is clearly documented to be "undefined", so it is completely within
> spec for it to be just about anything. One of the more likely bad
> scenarios is that the call internally allocates a buffer, starts to use
> it, hits some problem, then frees the buffer while leaving its address
> in place. Use of stale pointers should always be avoided.
>
> On 05/29/2015 09:48 AM, Bill Spitzak wrote:
>> I'm pretty certain the only two things that can happen is that str is
>> unchanged or it is set to NULL. So presetting it to NULL would work.
>>
>>
>> On Thu, May 28, 2015 at 7:01 PM, Jon A. Cruz <jonc at osg.samsung.com> wrote:
>>
>>> If asprintf fails for any reason, the contents of the pointer
>>> are undefined. While some platforms set it to NULL, there is no
>>> guarantee that all will.
>>>
>>> This change adds a simple wrapper to ensure proper NULL results
>>> on failure.
>>>
>>> Signed-off-by: Jon A. Cruz <jonc at osg.samsung.com>
>>> ---
>>> src/evdev-middle-button.c | 2 ++
>>> src/evdev-mt-touchpad-buttons.c | 2 ++
>>> src/evdev-mt-touchpad-edge-scroll.c | 2 ++
>>> src/libinput-util.h | 30 ++++++++++++++++++++++++++++++
>>> src/timer.c | 2 ++
>>> test/litest.c | 2 +-
>>> tools/libinput-list-devices.c | 14 +++++++-------
>>> 7 files changed, 46 insertions(+), 8 deletions(-)
>>>
>>> diff --git a/src/evdev-middle-button.c b/src/evdev-middle-button.c
>>> index 903fa4d..c989486 100644
>>> --- a/src/evdev-middle-button.c
>>> +++ b/src/evdev-middle-button.c
>>> @@ -20,6 +20,8 @@
>>> * CONNECTION WITH THE USE OR PERFORMANCE OF THIS SOFTWARE.
>>> */
>>>
>>> +#include "config.h"
>>> +
>>> #include <stdint.h>
>>>
>>> #include "evdev.h"
>>> diff --git a/src/evdev-mt-touchpad-buttons.c
>>> b/src/evdev-mt-touchpad-buttons.c
>>> index 64e9d28..4b1d6ab 100644
>>> --- a/src/evdev-mt-touchpad-buttons.c
>>> +++ b/src/evdev-mt-touchpad-buttons.c
>>> @@ -20,6 +20,8 @@
>>> * CONNECTION WITH THE USE OR PERFORMANCE OF THIS SOFTWARE.
>>> */
>>>
>>> +#include "config.h"
>>> +
>>> #include <errno.h>
>>> #include <limits.h>
>>> #include <math.h>
>>> diff --git a/src/evdev-mt-touchpad-edge-scroll.c
>>> b/src/evdev-mt-touchpad-edge-scroll.c
>>> index 585c764..2302d2c 100644
>>> --- a/src/evdev-mt-touchpad-edge-scroll.c
>>> +++ b/src/evdev-mt-touchpad-edge-scroll.c
>>> @@ -20,6 +20,8 @@
>>> * CONNECTION WITH THE USE OR PERFORMANCE OF THIS SOFTWARE.
>>> */
>>>
>>> +#include "config.h"
>>> +
>>> #include <errno.h>
>>> #include <limits.h>
>>> #include <math.h>
>>> diff --git a/src/libinput-util.h b/src/libinput-util.h
>>> index 732f813..7d5651a 100644
>>> --- a/src/libinput-util.h
>>> +++ b/src/libinput-util.h
>>> @@ -26,6 +26,8 @@
>>>
>>> #include <unistd.h>
>>> #include <math.h>
>>> +#include <stdarg.h>
>>> +#include <stdio.h>
>>> #include <string.h>
>>> #include <time.h>
>>>
>>> @@ -230,6 +232,34 @@ matrix_to_farray6(const struct matrix *m, float
>>> out[6])
>>> out[5] = m->val[1][2];
>>> }
>>>
>>> +/**
>>> + * Simple wrapper for asprintf that ensures the passed in pointer is set
>>> + * to NULL upon error.
>>> + * The standard asprintf() call does not guarantee the passed in pointer
>>> + * will be NULL'ed upon failure, whereas this wrapper does.
>>> + *
>>> + * @param strp pointer to set to newly allocated string.
>>> + * This pointer should be passed to free() to release when done.
>>> + * @param fmt the format string to use for printing.
>>> + * @return The number of bytes printed (excluding the null byte
>>> terminator)
>>> + * upon success or -1 upon failure. In the case of failure the pointer is
>>> set
>>> + * to NULL.
>>> + */
>>> +static inline int
>>> +xasprintf(char **strp, const char *fmt, ...)
>>> +{
>>> + int rc = 0;
>>> + va_list args;
>>> +
>>> + va_start(args, fmt);
>>> + rc = vasprintf(strp, fmt, args);
>>> + va_end(args);
>>> + if ((rc == -1) && strp)
>>> + *strp = NULL;
>>> +
>>> + return rc;
>>> +}
>>> +
>>> enum ratelimit_state {
>>> RATELIMIT_EXCEEDED,
>>> RATELIMIT_THRESHOLD,
>>> diff --git a/src/timer.c b/src/timer.c
>>> index dbd3894..1e507df 100644
>>> --- a/src/timer.c
>>> +++ b/src/timer.c
>>> @@ -20,6 +20,8 @@
>>> * CONNECTION WITH THE USE OR PERFORMANCE OF THIS SOFTWARE.
>>> */
>>>
>>> +#include "config.h"
>>> +
>>> #include <assert.h>
>>> #include <errno.h>
>>> #include <inttypes.h>
>>> diff --git a/test/litest.c b/test/litest.c
>>> index 53c441b..b3410bd 100644
>>> --- a/test/litest.c
>>> +++ b/test/litest.c
>>> @@ -876,7 +876,7 @@ litest_init_udev_rules(struct litest_test_device *dev)
>>> ck_abort_msg("Failed to create udev rules directory
>>> (%s)\n",
>>> strerror(errno));
>>>
>>> - rc = asprintf(&path,
>>> + rc = xasprintf(&path,
>>> "%s/%s%s.rules",
>>> UDEV_RULES_D,
>>> UDEV_RULE_PREFIX,
>>> diff --git a/tools/libinput-list-devices.c b/tools/libinput-list-devices.c
>>> index 6625173..68ddb61 100644
>>> --- a/tools/libinput-list-devices.c
>>> +++ b/tools/libinput-list-devices.c
>>> @@ -120,17 +120,17 @@ calibration_default(struct libinput_device *device)
>>> float calibration[6];
>>>
>>> if (!libinput_device_config_calibration_has_matrix(device)) {
>>> - asprintf(&str, "n/a");
>>> + xasprintf(&str, "n/a");
>>> return str;
>>> }
>>>
>>> if (libinput_device_config_calibration_get_default_matrix(device,
>>> calibration) == 0) {
>>> - asprintf(&str, "identity matrix");
>>> + xasprintf(&str, "identity matrix");
>>> return str;
>>> }
>>>
>>> - asprintf(&str,
>>> + xasprintf(&str,
>>> "%.2f %.2f %.2f %.2f %.2f %.2f",
>>> calibration[0],
>>> calibration[1],
>>> @@ -150,13 +150,13 @@ scroll_defaults(struct libinput_device *device)
>>>
>>> scroll_methods = libinput_device_config_scroll_get_methods(device);
>>> if (scroll_methods == LIBINPUT_CONFIG_SCROLL_NO_SCROLL) {
>>> - asprintf(&str, "none");
>>> + xasprintf(&str, "none");
>>> return str;
>>> }
>>>
>>> method = libinput_device_config_scroll_get_default_method(device);
>>>
>>> - asprintf(&str,
>>> + xasprintf(&str,
>>> "%s%s%s%s%s%s",
>>> (method == LIBINPUT_CONFIG_SCROLL_2FG) ? "*" : "",
>>> (scroll_methods & LIBINPUT_CONFIG_SCROLL_2FG) ?
>>> "two-finger " : "",
>>> @@ -176,12 +176,12 @@ click_defaults(struct libinput_device *device)
>>>
>>> click_methods = libinput_device_config_click_get_methods(device);
>>> if (click_methods == LIBINPUT_CONFIG_CLICK_METHOD_NONE) {
>>> - asprintf(&str, "none");
>>> + xasprintf(&str, "none");
>>> return str;
>>> }
>>>
>>> method = libinput_device_config_click_get_default_method(device);
>>> - asprintf(&str,
>>> + xasprintf(&str,
>>> "%s%s%s%s",
>>> (method == LIBINPUT_CONFIG_CLICK_METHOD_BUTTON_AREAS) ?
>>> "*" : "",
>>> (click_methods &
>>> LIBINPUT_CONFIG_CLICK_METHOD_BUTTON_AREAS) ? "button-areas " : "",
>>> --
>>> 2.1.0
>>>
>>> _______________________________________________
>>> wayland-devel mailing list
>>> wayland-devel at lists.freedesktop.org
>>> http://lists.freedesktop.org/mailman/listinfo/wayland-devel
>>>
>>
>
> --
> Jon A. Cruz - Senior Open Source Developer
> Samsung Open Source Group
> jonc at osg.samsung.com
> _______________________________________________
> wayland-devel mailing list
> wayland-devel at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/wayland-devel
--
Jasper
More information about the wayland-devel
mailing list