[PATCH libinput v2] Fix to avoid use of undefined pointer values.

Jon A. Cruz jonc at osg.samsung.com
Fri May 29 10:40:54 PDT 2015


Yes. The man page calls out its behavior.

http://linux.die.net/man/3/asprintf

> Return Value
> When successful, these functions return the number of bytes printed, just
> like sprintf(3). If memory allocation wasn't possible, or some other error
> occurs, these functions will return -1, and the contents of strp is
undefined.




On 05/29/2015 10:38 AM, Jasper St. Pierre wrote:
> 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
> 
> 
> 

-- 
Jon A. Cruz - Senior Open Source Developer
Samsung Open Source Group
jonc at osg.samsung.com


More information about the wayland-devel mailing list