[PATCH libinput v2] Fix to avoid use of undefined pointer values.
Jon A. Cruz
jonc at osg.samsung.com
Fri May 29 10:29:07 PDT 2015
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
More information about the wayland-devel
mailing list