* [PATCH bluez v2] monitor: fix buffer overflow when terminal width > 255
@ 2024-09-14 16:09 Celeste Liu
2024-09-14 17:42 ` [bluez,v2] " bluez.test.bot
2024-09-16 15:10 ` [PATCH bluez v2] " Luiz Augusto von Dentz
0 siblings, 2 replies; 7+ messages in thread
From: Celeste Liu @ 2024-09-14 16:09 UTC (permalink / raw)
To: Bluez; +Cc: Celeste Liu
In current code, we create line buffer with size 256, which can contains
255 ASCII characters. But in modern system, terminal can have larger
width. It may cause buffer overflow in snprintf() text.
We need allocate line buffer with size which can contains one line in
terminal. The size should be difficult to calculate because of multibyte
characters, but our code using line buffer assumed all characters has
1 byte size (e.g. when we put packet text into line buffer via
snprintf(), we calculate max size by 1B * col.), so it's safe to
allocate line buffer with col + 1.
Signed-off-by: Celeste Liu <CoelacanthusHex@gmail.com>
---
Changes in v2:
- Add free() forgot in v1.
- Link to v1: https://patch.msgid.link/20240914-fix-log-buffer-overflow-v1-1-733cb4fff673@gmail.com
---
monitor/packet.c | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/monitor/packet.c b/monitor/packet.c
index c2599fe6864ab44d657c121fcc3ceecc1ebc52a6..bef55477a221b6cb43ff224454ac3fa593cd8221 100644
--- a/monitor/packet.c
+++ b/monitor/packet.c
@@ -376,7 +376,8 @@ static void print_packet(struct timeval *tv, struct ucred *cred, char ident,
const char *text, const char *extra)
{
int col = num_columns();
- char line[256], ts_str[96], pid_str[140];
+ char ts_str[96], pid_str[140];
+ char *line = (char *) malloc(sizeof(char) * col + 1);
int n, ts_len = 0, ts_pos = 0, len = 0, pos = 0;
static size_t last_frame;
@@ -525,6 +526,7 @@ static void print_packet(struct timeval *tv, struct ucred *cred, char ident,
printf("%s%s\n", use_color() ? COLOR_TIMESTAMP : "", ts_str);
} else
printf("%s\n", line);
+ free(line);
}
static const struct {
---
base-commit: 41f943630d9a03c40e95057b2ac3d96470b9c71e
change-id: 20240914-fix-log-buffer-overflow-9aa5e61ee5b8
Best regards,
--
Celeste Liu <CoelacanthusHex@gmail.com>
^ permalink raw reply related [flat|nested] 7+ messages in thread
* RE: [bluez,v2] monitor: fix buffer overflow when terminal width > 255
2024-09-14 16:09 [PATCH bluez v2] monitor: fix buffer overflow when terminal width > 255 Celeste Liu
@ 2024-09-14 17:42 ` bluez.test.bot
2024-09-16 15:10 ` [PATCH bluez v2] " Luiz Augusto von Dentz
1 sibling, 0 replies; 7+ messages in thread
From: bluez.test.bot @ 2024-09-14 17:42 UTC (permalink / raw)
To: linux-bluetooth, coelacanthushex
[-- Attachment #1: Type: text/plain, Size: 2035 bytes --]
This is automated email and please do not reply to this email!
Dear submitter,
Thank you for submitting the patches to the linux bluetooth mailing list.
This is a CI test results with your patch series:
PW Link:https://patchwork.kernel.org/project/bluetooth/list/?series=890409
---Test result---
Test Summary:
CheckPatch PASS 0.49 seconds
GitLint FAIL 0.53 seconds
BuildEll PASS 24.64 seconds
BluezMake PASS 1646.76 seconds
MakeCheck PASS 13.26 seconds
MakeDistcheck PASS 178.85 seconds
CheckValgrind PASS 252.77 seconds
CheckSmatch WARNING 356.30 seconds
bluezmakeextell PASS 120.26 seconds
IncrementalBuild PASS 1413.47 seconds
ScanBuild PASS 1007.60 seconds
Details
##############################
Test: GitLint - FAIL
Desc: Run gitlint
Output:
[bluez,v2] monitor: fix buffer overflow when terminal width > 255
WARNING: I3 - ignore-body-lines: gitlint will be switching from using Python regex 'match' (match beginning) to 'search' (match anywhere) semantics. Please review your ignore-body-lines.regex option accordingly. To remove this warning, set general.regex-style-search=True. More details: https://jorisroovers.github.io/gitlint/configuration/#regex-style-search
18: B1 Line exceeds max length (99>80): "- Link to v1: https://patch.msgid.link/20240914-fix-log-buffer-overflow-v1-1-733cb4fff673@gmail.com"
##############################
Test: CheckSmatch - WARNING
Desc: Run smatch tool with source
Output:
monitor/packet.c: note: in included file:monitor/display.h:82:26: warning: Variable length array is used.monitor/packet.c:1869:26: warning: Variable length array is used.monitor/packet.c: note: in included file:monitor/bt.h:3606:52: warning: array of flexible structuresmonitor/bt.h:3594:40: warning: array of flexible structures
---
Regards,
Linux Bluetooth
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH bluez v2] monitor: fix buffer overflow when terminal width > 255
2024-09-14 16:09 [PATCH bluez v2] monitor: fix buffer overflow when terminal width > 255 Celeste Liu
2024-09-14 17:42 ` [bluez,v2] " bluez.test.bot
@ 2024-09-16 15:10 ` Luiz Augusto von Dentz
2024-09-16 15:39 ` Celeste Liu
1 sibling, 1 reply; 7+ messages in thread
From: Luiz Augusto von Dentz @ 2024-09-16 15:10 UTC (permalink / raw)
To: Celeste Liu; +Cc: Bluez
Hi Celeste,
On Sat, Sep 14, 2024 at 12:10 PM Celeste Liu <coelacanthushex@gmail.com> wrote:
>
> In current code, we create line buffer with size 256, which can contains
> 255 ASCII characters. But in modern system, terminal can have larger
> width. It may cause buffer overflow in snprintf() text.
>
> We need allocate line buffer with size which can contains one line in
> terminal. The size should be difficult to calculate because of multibyte
> characters, but our code using line buffer assumed all characters has
> 1 byte size (e.g. when we put packet text into line buffer via
> snprintf(), we calculate max size by 1B * col.), so it's safe to
> allocate line buffer with col + 1.
>
> Signed-off-by: Celeste Liu <CoelacanthusHex@gmail.com>
> ---
> Changes in v2:
> - Add free() forgot in v1.
> - Link to v1: https://patch.msgid.link/20240914-fix-log-buffer-overflow-v1-1-733cb4fff673@gmail.com
> ---
> monitor/packet.c | 4 +++-
> 1 file changed, 3 insertions(+), 1 deletion(-)
>
> diff --git a/monitor/packet.c b/monitor/packet.c
> index c2599fe6864ab44d657c121fcc3ceecc1ebc52a6..bef55477a221b6cb43ff224454ac3fa593cd8221 100644
> --- a/monitor/packet.c
> +++ b/monitor/packet.c
> @@ -376,7 +376,8 @@ static void print_packet(struct timeval *tv, struct ucred *cred, char ident,
> const char *text, const char *extra)
> {
> int col = num_columns();
> - char line[256], ts_str[96], pid_str[140];
> + char ts_str[96], pid_str[140];
> + char *line = (char *) malloc(sizeof(char) * col + 1);
Perhaps we could replace malloc with alloca here so we allocate the
line on the heap rather than stack.
> int n, ts_len = 0, ts_pos = 0, len = 0, pos = 0;
> static size_t last_frame;
>
> @@ -525,6 +526,7 @@ static void print_packet(struct timeval *tv, struct ucred *cred, char ident,
> printf("%s%s\n", use_color() ? COLOR_TIMESTAMP : "", ts_str);
> } else
> printf("%s\n", line);
> + free(line);
> }
>
> static const struct {
>
> ---
> base-commit: 41f943630d9a03c40e95057b2ac3d96470b9c71e
> change-id: 20240914-fix-log-buffer-overflow-9aa5e61ee5b8
>
> Best regards,
> --
> Celeste Liu <CoelacanthusHex@gmail.com>
>
>
--
Luiz Augusto von Dentz
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH bluez v2] monitor: fix buffer overflow when terminal width > 255
2024-09-16 15:10 ` [PATCH bluez v2] " Luiz Augusto von Dentz
@ 2024-09-16 15:39 ` Celeste Liu
2024-09-16 15:57 ` Celeste Liu
0 siblings, 1 reply; 7+ messages in thread
From: Celeste Liu @ 2024-09-16 15:39 UTC (permalink / raw)
To: Luiz Augusto von Dentz; +Cc: Bluez
On 2024-09-16 23:10, Luiz Augusto von Dentz wrote:
> Hi Celeste,
>
> On Sat, Sep 14, 2024 at 12:10 PM Celeste Liu <coelacanthushex@gmail.com> wrote:
>>
>> In current code, we create line buffer with size 256, which can contains
>> 255 ASCII characters. But in modern system, terminal can have larger
>> width. It may cause buffer overflow in snprintf() text.
>>
>> We need allocate line buffer with size which can contains one line in
>> terminal. The size should be difficult to calculate because of multibyte
>> characters, but our code using line buffer assumed all characters has
>> 1 byte size (e.g. when we put packet text into line buffer via
>> snprintf(), we calculate max size by 1B * col.), so it's safe to
>> allocate line buffer with col + 1.
>>
>> Signed-off-by: Celeste Liu <CoelacanthusHex@gmail.com>
>> ---
>> Changes in v2:
>> - Add free() forgot in v1.
>> - Link to v1: https://patch.msgid.link/20240914-fix-log-buffer-overflow-v1-1-733cb4fff673@gmail.com
>> ---
>> monitor/packet.c | 4 +++-
>> 1 file changed, 3 insertions(+), 1 deletion(-)
>>
>> diff --git a/monitor/packet.c b/monitor/packet.c
>> index c2599fe6864ab44d657c121fcc3ceecc1ebc52a6..bef55477a221b6cb43ff224454ac3fa593cd8221 100644
>> --- a/monitor/packet.c
>> +++ b/monitor/packet.c
>> @@ -376,7 +376,8 @@ static void print_packet(struct timeval *tv, struct ucred *cred, char ident,
>> const char *text, const char *extra)
>> {
>> int col = num_columns();
>> - char line[256], ts_str[96], pid_str[140];
>> + char ts_str[96], pid_str[140];
>> + char *line = (char *) malloc(sizeof(char) * col + 1);
>
> Perhaps we could replace malloc with alloca here so we allocate the
> line on the heap rather than stack.
I will replace it with alloca() in the next version.
But to be honest, I think alloca() is not a good choice. The compiler will
prevent the functions that call alloca() be inline, otherwise it may trigger
unexpected stack overflow because it's not a scope-based lifetime. It may be
better to replace it with VLA once we bump the standard requirement to C99 or
above.
>
>> int n, ts_len = 0, ts_pos = 0, len = 0, pos = 0;
>> static size_t last_frame;
>>
>> @@ -525,6 +526,7 @@ static void print_packet(struct timeval *tv, struct ucred *cred, char ident,
>> printf("%s%s\n", use_color() ? COLOR_TIMESTAMP : "", ts_str);
>> } else
>> printf("%s\n", line);
>> + free(line);
>> }
>>
>> static const struct {
>>
>> ---
>> base-commit: 41f943630d9a03c40e95057b2ac3d96470b9c71e
>> change-id: 20240914-fix-log-buffer-overflow-9aa5e61ee5b8
>>
>> Best regards,
>> --
>> Celeste Liu <CoelacanthusHex@gmail.com>
>>
>>
>
>
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH bluez v2] monitor: fix buffer overflow when terminal width > 255
2024-09-16 15:39 ` Celeste Liu
@ 2024-09-16 15:57 ` Celeste Liu
2024-09-16 16:22 ` Luiz Augusto von Dentz
0 siblings, 1 reply; 7+ messages in thread
From: Celeste Liu @ 2024-09-16 15:57 UTC (permalink / raw)
To: Luiz Augusto von Dentz; +Cc: Bluez, Marcel Holtmann
On 2024-09-16 23:39, Celeste Liu wrote:
>
> On 2024-09-16 23:10, Luiz Augusto von Dentz wrote:
>> Hi Celeste,
>>
>> On Sat, Sep 14, 2024 at 12:10 PM Celeste Liu <coelacanthushex@gmail.com> wrote:
>>>
>>> In current code, we create line buffer with size 256, which can contains
>>> 255 ASCII characters. But in modern system, terminal can have larger
>>> width. It may cause buffer overflow in snprintf() text.
>>>
>>> We need allocate line buffer with size which can contains one line in
>>> terminal. The size should be difficult to calculate because of multibyte
>>> characters, but our code using line buffer assumed all characters has
>>> 1 byte size (e.g. when we put packet text into line buffer via
>>> snprintf(), we calculate max size by 1B * col.), so it's safe to
>>> allocate line buffer with col + 1.
>>>
>>> Signed-off-by: Celeste Liu <CoelacanthusHex@gmail.com>
>>> ---
>>> Changes in v2:
>>> - Add free() forgot in v1.
>>> - Link to v1: https://patch.msgid.link/20240914-fix-log-buffer-overflow-v1-1-733cb4fff673@gmail.com
>>> ---
>>> monitor/packet.c | 4 +++-
>>> 1 file changed, 3 insertions(+), 1 deletion(-)
>>>
>>> diff --git a/monitor/packet.c b/monitor/packet.c
>>> index c2599fe6864ab44d657c121fcc3ceecc1ebc52a6..bef55477a221b6cb43ff224454ac3fa593cd8221 100644
>>> --- a/monitor/packet.c
>>> +++ b/monitor/packet.c
>>> @@ -376,7 +376,8 @@ static void print_packet(struct timeval *tv, struct ucred *cred, char ident,
>>> const char *text, const char *extra)
>>> {
>>> int col = num_columns();
>>> - char line[256], ts_str[96], pid_str[140];
>>> + char ts_str[96], pid_str[140];
>>> + char *line = (char *) malloc(sizeof(char) * col + 1);
>>
>> Perhaps we could replace malloc with alloca here so we allocate the
>> line on the heap rather than stack.
>
> I will replace it with alloca() in the next version.
> But to be honest, I think alloca() is not a good choice. The compiler will
> prevent the functions that call alloca() be inline, otherwise it may trigger
> unexpected stack overflow because it's not a scope-based lifetime. It may be
> better to replace it with VLA once we bump the standard requirement to C99 or
> above.
But I found a VLA usage in monitor/display.h:82, which was introduced by Marcel Holtmann
in d9e3aab39d2af7d7a822993ededaa41cd0311c53 in 2012. Could we use VLA directly? Or we
need to treat that usage as a bug and fix it?
>
>>
>>> int n, ts_len = 0, ts_pos = 0, len = 0, pos = 0;
>>> static size_t last_frame;
>>>
>>> @@ -525,6 +526,7 @@ static void print_packet(struct timeval *tv, struct ucred *cred, char ident,
>>> printf("%s%s\n", use_color() ? COLOR_TIMESTAMP : "", ts_str);
>>> } else
>>> printf("%s\n", line);
>>> + free(line);
>>> }
>>>
>>> static const struct {
>>>
>>> ---
>>> base-commit: 41f943630d9a03c40e95057b2ac3d96470b9c71e
>>> change-id: 20240914-fix-log-buffer-overflow-9aa5e61ee5b8
>>>
>>> Best regards,
>>> --
>>> Celeste Liu <CoelacanthusHex@gmail.com>
>>>
>>>
>>
>>
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH bluez v2] monitor: fix buffer overflow when terminal width > 255
2024-09-16 15:57 ` Celeste Liu
@ 2024-09-16 16:22 ` Luiz Augusto von Dentz
2024-09-17 6:31 ` Celeste Liu
0 siblings, 1 reply; 7+ messages in thread
From: Luiz Augusto von Dentz @ 2024-09-16 16:22 UTC (permalink / raw)
To: Celeste Liu; +Cc: Bluez, Marcel Holtmann
Hi,
On Mon, Sep 16, 2024 at 11:57 AM Celeste Liu <coelacanthushex@gmail.com> wrote:
>
>
> On 2024-09-16 23:39, Celeste Liu wrote:
> >
> > On 2024-09-16 23:10, Luiz Augusto von Dentz wrote:
> >> Hi Celeste,
> >>
> >> On Sat, Sep 14, 2024 at 12:10 PM Celeste Liu <coelacanthushex@gmail.com> wrote:
> >>>
> >>> In current code, we create line buffer with size 256, which can contains
> >>> 255 ASCII characters. But in modern system, terminal can have larger
> >>> width. It may cause buffer overflow in snprintf() text.
> >>>
> >>> We need allocate line buffer with size which can contains one line in
> >>> terminal. The size should be difficult to calculate because of multibyte
> >>> characters, but our code using line buffer assumed all characters has
> >>> 1 byte size (e.g. when we put packet text into line buffer via
> >>> snprintf(), we calculate max size by 1B * col.), so it's safe to
> >>> allocate line buffer with col + 1.
> >>>
> >>> Signed-off-by: Celeste Liu <CoelacanthusHex@gmail.com>
> >>> ---
> >>> Changes in v2:
> >>> - Add free() forgot in v1.
> >>> - Link to v1: https://patch.msgid.link/20240914-fix-log-buffer-overflow-v1-1-733cb4fff673@gmail.com
> >>> ---
> >>> monitor/packet.c | 4 +++-
> >>> 1 file changed, 3 insertions(+), 1 deletion(-)
> >>>
> >>> diff --git a/monitor/packet.c b/monitor/packet.c
> >>> index c2599fe6864ab44d657c121fcc3ceecc1ebc52a6..bef55477a221b6cb43ff224454ac3fa593cd8221 100644
> >>> --- a/monitor/packet.c
> >>> +++ b/monitor/packet.c
> >>> @@ -376,7 +376,8 @@ static void print_packet(struct timeval *tv, struct ucred *cred, char ident,
> >>> const char *text, const char *extra)
> >>> {
> >>> int col = num_columns();
> >>> - char line[256], ts_str[96], pid_str[140];
> >>> + char ts_str[96], pid_str[140];
> >>> + char *line = (char *) malloc(sizeof(char) * col + 1);
> >>
> >> Perhaps we could replace malloc with alloca here so we allocate the
> >> line on the heap rather than stack.
> >
> > I will replace it with alloca() in the next version.
> > But to be honest, I think alloca() is not a good choice. The compiler will
> > prevent the functions that call alloca() be inline, otherwise it may trigger
> > unexpected stack overflow because it's not a scope-based lifetime. It may be
> > better to replace it with VLA once we bump the standard requirement to C99 or
> > above.
>
> But I found a VLA usage in monitor/display.h:82, which was introduced by Marcel Holtmann
> in d9e3aab39d2af7d7a822993ededaa41cd0311c53 in 2012. Could we use VLA directly? Or we
> need to treat that usage as a bug and fix it?
Afaik VLA has more problems than using alloca, because depending on
the C (11-23) version VLA is optional, so afaik alloca is a safer
option provided the length is not considered too big, anyway perhaps
we should use some define like LINE_MAX instead:
{LINE_MAX}
Unless otherwise noted, the maximum length, in bytes, of a
utility's input line (either standard input or another
file), when the utility is described as processing text
files. The length includes room for the trailing <newline>.
Minimum Acceptable Value: {_POSIX2_LINE_MAX}
https://www.man7.org/linux/man-pages/man0/limits.h.0p.html
So something like the following:
diff --git a/monitor/packet.c b/monitor/packet.c
index c2599fe6864a..32a440bbea68 100644
--- a/monitor/packet.c
+++ b/monitor/packet.c
@@ -26,6 +26,7 @@
#include <time.h>
#include <sys/time.h>
#include <sys/socket.h>
+#include <limits.h>
#include "lib/bluetooth.h"
#include "lib/uuid.h"
@@ -376,7 +377,7 @@ static void print_packet(struct timeval *tv,
struct ucred *cred, char ident,
const char *text, const char *extra)
{
int col = num_columns();
- char line[256], ts_str[96], pid_str[140];
+ char line[LINE_MAX], ts_str[96], pid_str[140];
int n, ts_len = 0, ts_pos = 0, len = 0, pos = 0;
static size_t last_frame;
> >
> >>
> >>> int n, ts_len = 0, ts_pos = 0, len = 0, pos = 0;
> >>> static size_t last_frame;
> >>>
> >>> @@ -525,6 +526,7 @@ static void print_packet(struct timeval *tv, struct ucred *cred, char ident,
> >>> printf("%s%s\n", use_color() ? COLOR_TIMESTAMP : "", ts_str);
> >>> } else
> >>> printf("%s\n", line);
> >>> + free(line);
> >>> }
> >>>
> >>> static const struct {
> >>>
> >>> ---
> >>> base-commit: 41f943630d9a03c40e95057b2ac3d96470b9c71e
> >>> change-id: 20240914-fix-log-buffer-overflow-9aa5e61ee5b8
> >>>
> >>> Best regards,
> >>> --
> >>> Celeste Liu <CoelacanthusHex@gmail.com>
> >>>
> >>>
> >>
> >>
>
--
Luiz Augusto von Dentz
^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [PATCH bluez v2] monitor: fix buffer overflow when terminal width > 255
2024-09-16 16:22 ` Luiz Augusto von Dentz
@ 2024-09-17 6:31 ` Celeste Liu
0 siblings, 0 replies; 7+ messages in thread
From: Celeste Liu @ 2024-09-17 6:31 UTC (permalink / raw)
To: Luiz Augusto von Dentz; +Cc: Bluez, Marcel Holtmann
On 2024-09-17 00:22, Luiz Augusto von Dentz wrote:
> Hi,
>
> On Mon, Sep 16, 2024 at 11:57 AM Celeste Liu <coelacanthushex@gmail.com> wrote:
>>
>>
>> On 2024-09-16 23:39, Celeste Liu wrote:
>>>
>>> On 2024-09-16 23:10, Luiz Augusto von Dentz wrote:
>>>> Hi Celeste,
>>>>
>>>> On Sat, Sep 14, 2024 at 12:10 PM Celeste Liu <coelacanthushex@gmail.com> wrote:
>>>>>
>>>>> In current code, we create line buffer with size 256, which can contains
>>>>> 255 ASCII characters. But in modern system, terminal can have larger
>>>>> width. It may cause buffer overflow in snprintf() text.
>>>>>
>>>>> We need allocate line buffer with size which can contains one line in
>>>>> terminal. The size should be difficult to calculate because of multibyte
>>>>> characters, but our code using line buffer assumed all characters has
>>>>> 1 byte size (e.g. when we put packet text into line buffer via
>>>>> snprintf(), we calculate max size by 1B * col.), so it's safe to
>>>>> allocate line buffer with col + 1.
>>>>>
>>>>> Signed-off-by: Celeste Liu <CoelacanthusHex@gmail.com>
>>>>> ---
>>>>> Changes in v2:
>>>>> - Add free() forgot in v1.
>>>>> - Link to v1: https://patch.msgid.link/20240914-fix-log-buffer-overflow-v1-1-733cb4fff673@gmail.com
>>>>> ---
>>>>> monitor/packet.c | 4 +++-
>>>>> 1 file changed, 3 insertions(+), 1 deletion(-)
>>>>>
>>>>> diff --git a/monitor/packet.c b/monitor/packet.c
>>>>> index c2599fe6864ab44d657c121fcc3ceecc1ebc52a6..bef55477a221b6cb43ff224454ac3fa593cd8221 100644
>>>>> --- a/monitor/packet.c
>>>>> +++ b/monitor/packet.c
>>>>> @@ -376,7 +376,8 @@ static void print_packet(struct timeval *tv, struct ucred *cred, char ident,
>>>>> const char *text, const char *extra)
>>>>> {
>>>>> int col = num_columns();
>>>>> - char line[256], ts_str[96], pid_str[140];
>>>>> + char ts_str[96], pid_str[140];
>>>>> + char *line = (char *) malloc(sizeof(char) * col + 1);
>>>>
>>>> Perhaps we could replace malloc with alloca here so we allocate the
>>>> line on the heap rather than stack.
>>>
>>> I will replace it with alloca() in the next version.
>>> But to be honest, I think alloca() is not a good choice. The compiler will
>>> prevent the functions that call alloca() be inline, otherwise it may trigger
>>> unexpected stack overflow because it's not a scope-based lifetime. It may be
>>> better to replace it with VLA once we bump the standard requirement to C99 or
>>> above.
>>
>> But I found a VLA usage in monitor/display.h:82, which was introduced by Marcel Holtmann
>> in d9e3aab39d2af7d7a822993ededaa41cd0311c53 in 2012. Could we use VLA directly? Or we
>> need to treat that usage as a bug and fix it?
>
> Afaik VLA has more problems than using alloca, because depending on
> the C (11-23) version VLA is optional, so afaik alloca is a safer
> option provided the length is not considered too big, anyway perhaps
> we should use some define like LINE_MAX instead:
>
> {LINE_MAX}
> Unless otherwise noted, the maximum length, in bytes, of a
> utility's input line (either standard input or another
> file), when the utility is described as processing text
> files. The length includes room for the trailing <newline>.
> Minimum Acceptable Value: {_POSIX2_LINE_MAX}
> https://www.man7.org/linux/man-pages/man0/limits.h.0p.html
v3 has been sent.
> So something like the following:
>
> diff --git a/monitor/packet.c b/monitor/packet.c
> index c2599fe6864a..32a440bbea68 100644
> --- a/monitor/packet.c
> +++ b/monitor/packet.c
> @@ -26,6 +26,7 @@
> #include <time.h>
> #include <sys/time.h>
> #include <sys/socket.h>
> +#include <limits.h>
>
> #include "lib/bluetooth.h"
> #include "lib/uuid.h"
> @@ -376,7 +377,7 @@ static void print_packet(struct timeval *tv,
> struct ucred *cred, char ident,
> const char *text, const char *extra)
> {
> int col = num_columns();
> - char line[256], ts_str[96], pid_str[140];
> + char line[LINE_MAX], ts_str[96], pid_str[140];
> int n, ts_len = 0, ts_pos = 0, len = 0, pos = 0;
> static size_t last_frame;
>
>>>
>>>>
>>>>> int n, ts_len = 0, ts_pos = 0, len = 0, pos = 0;
>>>>> static size_t last_frame;
>>>>>
>>>>> @@ -525,6 +526,7 @@ static void print_packet(struct timeval *tv, struct ucred *cred, char ident,
>>>>> printf("%s%s\n", use_color() ? COLOR_TIMESTAMP : "", ts_str);
>>>>> } else
>>>>> printf("%s\n", line);
>>>>> + free(line);
>>>>> }
>>>>>
>>>>> static const struct {
>>>>>
>>>>> ---
>>>>> base-commit: 41f943630d9a03c40e95057b2ac3d96470b9c71e
>>>>> change-id: 20240914-fix-log-buffer-overflow-9aa5e61ee5b8
>>>>>
>>>>> Best regards,
>>>>> --
>>>>> Celeste Liu <CoelacanthusHex@gmail.com>
>>>>>
>>>>>
>>>>
>>>>
>>
>
>
^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2024-09-17 6:31 UTC | newest]
Thread overview: 7+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2024-09-14 16:09 [PATCH bluez v2] monitor: fix buffer overflow when terminal width > 255 Celeste Liu
2024-09-14 17:42 ` [bluez,v2] " bluez.test.bot
2024-09-16 15:10 ` [PATCH bluez v2] " Luiz Augusto von Dentz
2024-09-16 15:39 ` Celeste Liu
2024-09-16 15:57 ` Celeste Liu
2024-09-16 16:22 ` Luiz Augusto von Dentz
2024-09-17 6:31 ` Celeste Liu
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox