Fix potential NULL pointer dereference
Commit Message
This patch addresses a potential NULL pointer dereference when we try to
duplicate a string. The input pointer can be NULL and that may lead to
crashes. We just use a statically-allocated string to prevent bad things from
happening.
gdb/ChangeLog:
2016-10-24 Luis Machado <lgustavo@codesourcery.com>
* exec.c (exec_file_locate_attach): Prevent NULL pointer dereference
when duplicating a string.
---
gdb/ChangeLog | 5 +++++
gdb/exec.c | 11 +++++++++--
2 files changed, 14 insertions(+), 2 deletions(-)
Comments
On 10/24/2016 10:52 PM, Luis Machado wrote:
> diff --git a/gdb/exec.c b/gdb/exec.c
> index 67ecc63..5eeac44 100644
> --- a/gdb/exec.c
> +++ b/gdb/exec.c
> @@ -221,13 +221,20 @@ exec_file_locate_attach (int pid, int defer_bp_reset, int from_tty)
> }
> CATCH (err, RETURN_MASK_ERROR)
> {
> + const char *msg;
> +
> if (err.message != NULL)
> - warning ("%s", err.message);
> + {
> + warning ("%s", err.message);
> + msg = err.message;
> + }
> + else
> + msg = "";
>
This is overcomplicating a bit, IMO.
> prev_err = err;
>
> /* Save message so it doesn't get trashed by the catch below. */
> - prev_err.message = xstrdup (err.message);
> + prev_err.message = xstrdup (msg);
I think this one liner:
+ if (err.message != NULL)
prev_err.message = xstrdup (err.message);
would have the same effect, since exception_print_same
handles NULL ex.message?
Thanks,
Pedro Alves
On 10/24/2016 05:26 PM, Pedro Alves wrote:
> On 10/24/2016 10:52 PM, Luis Machado wrote:
>
>> diff --git a/gdb/exec.c b/gdb/exec.c
>> index 67ecc63..5eeac44 100644
>> --- a/gdb/exec.c
>> +++ b/gdb/exec.c
>> @@ -221,13 +221,20 @@ exec_file_locate_attach (int pid, int defer_bp_reset, int from_tty)
>> }
>> CATCH (err, RETURN_MASK_ERROR)
>> {
>> + const char *msg;
>> +
>> if (err.message != NULL)
>> - warning ("%s", err.message);
>> + {
>> + warning ("%s", err.message);
>> + msg = err.message;
>> + }
>> + else
>> + msg = "";
>>
>
> This is overcomplicating a bit, IMO.
>
>> prev_err = err;
>>
>> /* Save message so it doesn't get trashed by the catch below. */
>> - prev_err.message = xstrdup (err.message);
>> + prev_err.message = xstrdup (msg);
>
>
> I think this one liner:
>
> + if (err.message != NULL)
> prev_err.message = xstrdup (err.message);
>
> would have the same effect, since exception_print_same
> handles NULL ex.message?
>
> Thanks,
> Pedro Alves
>
Fine by me. I'll push that change then.
On 10/24/2016 05:39 PM, Luis Machado wrote:
> On 10/24/2016 05:26 PM, Pedro Alves wrote:
>> On 10/24/2016 10:52 PM, Luis Machado wrote:
>>
>>> diff --git a/gdb/exec.c b/gdb/exec.c
>>> index 67ecc63..5eeac44 100644
>>> --- a/gdb/exec.c
>>> +++ b/gdb/exec.c
>>> @@ -221,13 +221,20 @@ exec_file_locate_attach (int pid, int
>>> defer_bp_reset, int from_tty)
>>> }
>>> CATCH (err, RETURN_MASK_ERROR)
>>> {
>>> + const char *msg;
>>> +
>>> if (err.message != NULL)
>>> - warning ("%s", err.message);
>>> + {
>>> + warning ("%s", err.message);
>>> + msg = err.message;
>>> + }
>>> + else
>>> + msg = "";
>>>
>>
>> This is overcomplicating a bit, IMO.
>>
>>> prev_err = err;
>>>
>>> /* Save message so it doesn't get trashed by the catch below. */
>>> - prev_err.message = xstrdup (err.message);
>>> + prev_err.message = xstrdup (msg);
>>
>>
>> I think this one liner:
>>
>> + if (err.message != NULL)
>> prev_err.message = xstrdup (err.message);
>>
>> would have the same effect, since exception_print_same
>> handles NULL ex.message?
>>
>> Thanks,
>> Pedro Alves
>>
>
> Fine by me. I'll push that change then.
>
Pushed as b5e1db87897cabfd9beb8b1bd49f7d965c0f2607 now.
Thanks.
@@ -1,5 +1,10 @@
2016-10-24 Luis Machado <lgustavo@codesourcery.com>
+ * exec.c (exec_file_locate_attach): Prevent NULL pointer dereference
+ when duplicating a string.
+
+2016-10-24 Luis Machado <lgustavo@codesourcery.com>
+
* exec.c (exception_print_same): Fix string comparison to use
statically-allocated ones.
@@ -221,13 +221,20 @@ exec_file_locate_attach (int pid, int defer_bp_reset, int from_tty)
}
CATCH (err, RETURN_MASK_ERROR)
{
+ const char *msg;
+
if (err.message != NULL)
- warning ("%s", err.message);
+ {
+ warning ("%s", err.message);
+ msg = err.message;
+ }
+ else
+ msg = "";
prev_err = err;
/* Save message so it doesn't get trashed by the catch below. */
- prev_err.message = xstrdup (err.message);
+ prev_err.message = xstrdup (msg);
}
END_CATCH