* [dpdk-dev] [PATCH] librte_cmdline: FreeBSD Fix oveflow when size of command result structure is greater than BUFSIZ
@ 2014-10-20 15:23 Alan Carew
2014-10-20 15:26 ` Carew, Alan
0 siblings, 1 reply; 9+ messages in thread
From: Alan Carew @ 2014-10-20 15:23 UTC (permalink / raw)
To: dev
When using test-pmd with flow director in FreeBSD, the application will
segfault/Bus error while parsing the command-line. This is due to how
each commands result structure is represented during parsing, where the offsets
for each tokens value is stored in a character array(char result_buf[BUFSIZ])
in cmdline_parse()(./lib/librte_cmdline/cmdline_parse.c).
The overflow occurs where BUFSIZ is less than the size of a commands result
structure, in this case "struct cmd_pkt_filter_result"
(app/test-pmd/cmdline.c) is 1088 bytes and BUFSIZ on FreeBSD is 1024 bytes as
opposed to 8192 bytes on Linux.
This patch removes the OS dependency on BUFSIZ and defines and uses a
library #define CMDLINE_PARSE_RESULT_BUFSIZE 8192
The problem can be reproduced by running test-pmd on FreeBSD:
./testpmd -c 0x3 -n 4 -- -i --portmask=0x3 --pkt-filter-mode=perfect
And adding a filter:
add_perfect_filter 0 udp src 192.168.0.0 1024 dst 192.168.0.0 1024 flexbytes
0x800 vlan 0 queue 0 soft 0x17
Signed-off-by: Alan Carew <alan.carew@intel.com>
---
lib/librte_cmdline/cmdline_parse.c | 2 +-
lib/librte_cmdline/cmdline_parse.h | 3 +++
2 files changed, 4 insertions(+), 1 deletion(-)
diff --git a/lib/librte_cmdline/cmdline_parse.c b/lib/librte_cmdline/cmdline_parse.c
index 940480d..29f1afd 100644
--- a/lib/librte_cmdline/cmdline_parse.c
+++ b/lib/librte_cmdline/cmdline_parse.c
@@ -219,7 +219,7 @@ cmdline_parse(struct cmdline *cl, const char * buf)
unsigned int inst_num=0;
cmdline_parse_inst_t *inst;
const char *curbuf;
- char result_buf[BUFSIZ];
+ char result_buf[CMDLINE_PARSE_RESULT_BUFSIZE];
void (*f)(void *, struct cmdline *, void *) = NULL;
void *data = NULL;
int comment = 0;
diff --git a/lib/librte_cmdline/cmdline_parse.h b/lib/librte_cmdline/cmdline_parse.h
index f18836d..dae53ba 100644
--- a/lib/librte_cmdline/cmdline_parse.h
+++ b/lib/librte_cmdline/cmdline_parse.h
@@ -80,6 +80,9 @@ extern "C" {
#define CMDLINE_PARSE_COMPLETE_AGAIN 1
#define CMDLINE_PARSE_COMPLETED_BUFFER 2
+/* maximum buffer size for parsed result */
+#define CMDLINE_PARSE_RESULT_BUFSIZE 8192
+
/**
* Stores a pointer to the ops struct, and the offset: the place to
* write the parsed result in the destination structure.
--
1.9.3
^ permalink raw reply [flat|nested] 9+ messages in thread* Re: [dpdk-dev] [PATCH] librte_cmdline: FreeBSD Fix oveflow when size of command result structure is greater than BUFSIZ
2014-10-20 15:23 [dpdk-dev] [PATCH] librte_cmdline: FreeBSD Fix oveflow when size of command result structure is greater than BUFSIZ Alan Carew
@ 2014-10-20 15:26 ` Carew, Alan
2014-10-20 20:25 ` Thomas Monjalon
2014-10-27 9:14 ` Olivier MATZ
0 siblings, 2 replies; 9+ messages in thread
From: Carew, Alan @ 2014-10-20 15:26 UTC (permalink / raw)
To: dev
A comment on my own patch.
Making the size of result_buf consistent across each OS and keeping it as large
as the Linux BUFSIZ(8192) doesn't really address the core issue.
In the event that a user of librte_cmdline creates a custom context with a
result structure > 8192 bytes then this problem will occur again, though
somewhat unlikely, as the minimum number of the largest type would be 64 x
cmdline_fixed_string_t types within a result structure, at its current size.
There is no checking of overflow, I would be tempted to add a runtime check in
cmdline_parse()/match_inst(), however I would be more comfortable with a build
time check for this type of problem.
Due to the opaque handling of user defined contexts there is no obvious way to
do this at build time.
Thoughts?
> -----Original Message-----
> From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Alan Carew
> Sent: Monday, October 20, 2014 4:23 PM
> To: dev@dpdk.org
> Subject: [dpdk-dev] [PATCH] librte_cmdline: FreeBSD Fix oveflow when size of
> command result structure is greater than BUFSIZ
>
> When using test-pmd with flow director in FreeBSD, the application will
> segfault/Bus error while parsing the command-line. This is due to how
> each commands result structure is represented during parsing, where the offsets
> for each tokens value is stored in a character array(char result_buf[BUFSIZ])
> in cmdline_parse()(./lib/librte_cmdline/cmdline_parse.c).
>
> The overflow occurs where BUFSIZ is less than the size of a commands result
> structure, in this case "struct cmd_pkt_filter_result"
> (app/test-pmd/cmdline.c) is 1088 bytes and BUFSIZ on FreeBSD is 1024 bytes as
> opposed to 8192 bytes on Linux.
>
> This patch removes the OS dependency on BUFSIZ and defines and uses a
> library #define CMDLINE_PARSE_RESULT_BUFSIZE 8192
>
> The problem can be reproduced by running test-pmd on FreeBSD:
> ./testpmd -c 0x3 -n 4 -- -i --portmask=0x3 --pkt-filter-mode=perfect
> And adding a filter:
> add_perfect_filter 0 udp src 192.168.0.0 1024 dst 192.168.0.0 1024 flexbytes
> 0x800 vlan 0 queue 0 soft 0x17
>
> Signed-off-by: Alan Carew <alan.carew@intel.com>
> ---
> lib/librte_cmdline/cmdline_parse.c | 2 +-
> lib/librte_cmdline/cmdline_parse.h | 3 +++
> 2 files changed, 4 insertions(+), 1 deletion(-)
>
> diff --git a/lib/librte_cmdline/cmdline_parse.c
> b/lib/librte_cmdline/cmdline_parse.c
> index 940480d..29f1afd 100644
> --- a/lib/librte_cmdline/cmdline_parse.c
> +++ b/lib/librte_cmdline/cmdline_parse.c
> @@ -219,7 +219,7 @@ cmdline_parse(struct cmdline *cl, const char * buf)
> unsigned int inst_num=0;
> cmdline_parse_inst_t *inst;
> const char *curbuf;
> - char result_buf[BUFSIZ];
> + char result_buf[CMDLINE_PARSE_RESULT_BUFSIZE];
> void (*f)(void *, struct cmdline *, void *) = NULL;
> void *data = NULL;
> int comment = 0;
> diff --git a/lib/librte_cmdline/cmdline_parse.h
> b/lib/librte_cmdline/cmdline_parse.h
> index f18836d..dae53ba 100644
> --- a/lib/librte_cmdline/cmdline_parse.h
> +++ b/lib/librte_cmdline/cmdline_parse.h
> @@ -80,6 +80,9 @@ extern "C" {
> #define CMDLINE_PARSE_COMPLETE_AGAIN 1
> #define CMDLINE_PARSE_COMPLETED_BUFFER 2
>
> +/* maximum buffer size for parsed result */
> +#define CMDLINE_PARSE_RESULT_BUFSIZE 8192
> +
> /**
> * Stores a pointer to the ops struct, and the offset: the place to
> * write the parsed result in the destination structure.
> --
> 1.9.3
^ permalink raw reply [flat|nested] 9+ messages in thread* Re: [dpdk-dev] [PATCH] librte_cmdline: FreeBSD Fix oveflow when size of command result structure is greater than BUFSIZ
2014-10-20 15:26 ` Carew, Alan
@ 2014-10-20 20:25 ` Thomas Monjalon
2014-10-27 9:14 ` Olivier MATZ
1 sibling, 0 replies; 9+ messages in thread
From: Thomas Monjalon @ 2014-10-20 20:25 UTC (permalink / raw)
To: Carew, Alan; +Cc: dev
Hi Alan,
2014-10-20 15:26, Carew, Alan:
> A comment on my own patch.
>
> Making the size of result_buf consistent across each OS and keeping it as large
> as the Linux BUFSIZ(8192) doesn't really address the core issue.
>
> In the event that a user of librte_cmdline creates a custom context with a
> result structure > 8192 bytes then this problem will occur again, though
> somewhat unlikely, as the minimum number of the largest type would be 64 x
> cmdline_fixed_string_t types within a result structure, at its current size.
>
> There is no checking of overflow, I would be tempted to add a runtime check in
> cmdline_parse()/match_inst(), however I would be more comfortable with a build
> time check for this type of problem.
>
> Due to the opaque handling of user defined contexts there is no obvious way to
> do this at build time.
>
> Thoughts?
librte_cmdline derivates from libcmdline written by Olivier Matz:
http://git.droids-corp.org/?p=libcmdline.git
Maybe there are some fixes to take here, and probably Olivier will have
some good insights.
> > -----Original Message-----
> > From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Alan Carew
> > Sent: Monday, October 20, 2014 4:23 PM
> > To: dev@dpdk.org
> > Subject: [dpdk-dev] [PATCH] librte_cmdline: FreeBSD Fix oveflow when size of
> > command result structure is greater than BUFSIZ
> >
> > When using test-pmd with flow director in FreeBSD, the application will
> > segfault/Bus error while parsing the command-line. This is due to how
> > each commands result structure is represented during parsing, where the offsets
> > for each tokens value is stored in a character array(char result_buf[BUFSIZ])
> > in cmdline_parse()(./lib/librte_cmdline/cmdline_parse.c).
> >
> > The overflow occurs where BUFSIZ is less than the size of a commands result
> > structure, in this case "struct cmd_pkt_filter_result"
> > (app/test-pmd/cmdline.c) is 1088 bytes and BUFSIZ on FreeBSD is 1024 bytes as
> > opposed to 8192 bytes on Linux.
> >
> > This patch removes the OS dependency on BUFSIZ and defines and uses a
> > library #define CMDLINE_PARSE_RESULT_BUFSIZE 8192
> >
> > The problem can be reproduced by running test-pmd on FreeBSD:
> > ./testpmd -c 0x3 -n 4 -- -i --portmask=0x3 --pkt-filter-mode=perfect
> > And adding a filter:
> > add_perfect_filter 0 udp src 192.168.0.0 1024 dst 192.168.0.0 1024 flexbytes
> > 0x800 vlan 0 queue 0 soft 0x17
> >
> > Signed-off-by: Alan Carew <alan.carew@intel.com>
> > ---
> > lib/librte_cmdline/cmdline_parse.c | 2 +-
> > lib/librte_cmdline/cmdline_parse.h | 3 +++
> > 2 files changed, 4 insertions(+), 1 deletion(-)
[...]
> > --- a/lib/librte_cmdline/cmdline_parse.c
> > +++ b/lib/librte_cmdline/cmdline_parse.c
> > @@ -219,7 +219,7 @@ cmdline_parse(struct cmdline *cl, const char * buf)
> > unsigned int inst_num=0;
> > cmdline_parse_inst_t *inst;
> > const char *curbuf;
> > - char result_buf[BUFSIZ];
> > + char result_buf[CMDLINE_PARSE_RESULT_BUFSIZE];
[...]
> > --- a/lib/librte_cmdline/cmdline_parse.h
> > +++ b/lib/librte_cmdline/cmdline_parse.h
> > @@ -80,6 +80,9 @@ extern "C" {
> > #define CMDLINE_PARSE_COMPLETE_AGAIN 1
> > #define CMDLINE_PARSE_COMPLETED_BUFFER 2
> >
> > +/* maximum buffer size for parsed result */
> > +#define CMDLINE_PARSE_RESULT_BUFSIZE 8192
> > +
^ permalink raw reply [flat|nested] 9+ messages in thread* Re: [dpdk-dev] [PATCH] librte_cmdline: FreeBSD Fix oveflow when size of command result structure is greater than BUFSIZ
2014-10-20 15:26 ` Carew, Alan
2014-10-20 20:25 ` Thomas Monjalon
@ 2014-10-27 9:14 ` Olivier MATZ
2014-12-03 16:05 ` Olivier MATZ
1 sibling, 1 reply; 9+ messages in thread
From: Olivier MATZ @ 2014-10-27 9:14 UTC (permalink / raw)
To: Carew, Alan, dev
Hello Alan,
On 10/20/2014 05:26 PM, Carew, Alan wrote:
> A comment on my own patch.
>
> Making the size of result_buf consistent across each OS and keeping it as large
> as the Linux BUFSIZ(8192) doesn't really address the core issue.
>
> In the event that a user of librte_cmdline creates a custom context with a
> result structure > 8192 bytes then this problem will occur again, though
> somewhat unlikely, as the minimum number of the largest type would be 64 x
> cmdline_fixed_string_t types within a result structure, at its current size.
>
> There is no checking of overflow, I would be tempted to add a runtime check in
> cmdline_parse()/match_inst(), however I would be more comfortable with a build
> time check for this type of problem.
>
> Due to the opaque handling of user defined contexts there is no obvious way to
> do this at build time.
>
> Thoughts?
Indeed, your patch does not address the core issue of the problem,
altough it's already an improvement to the current situation.
Your issue was already fixed in the latest libcmdline library by
this patch (which also includes the replacement of BUFSIZ):
http://git.droids-corp.org/?p=libcmdline.git;a=commitdiff;h=b1d5b169352e57df3fc14c51ffad4b83f3e5613f
I'm pretty sure it won't apply smoothly on the dpdk command line
library but it can probably be adapted. Ideally, the latest libcmdline
library should be [cleaned first and] merged in dpdk.org.
Regards,
Olivier
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [dpdk-dev] [PATCH] librte_cmdline: FreeBSD Fix oveflow when size of command result structure is greater than BUFSIZ
2014-10-27 9:14 ` Olivier MATZ
@ 2014-12-03 16:05 ` Olivier MATZ
2014-12-03 18:12 ` Thomas Monjalon
0 siblings, 1 reply; 9+ messages in thread
From: Olivier MATZ @ 2014-12-03 16:05 UTC (permalink / raw)
To: Carew, Alan, dev
Hi,
On 10/27/2014 10:14 AM, Olivier MATZ wrote:
> Hello Alan,
>
> On 10/20/2014 05:26 PM, Carew, Alan wrote:
>> A comment on my own patch.
>>
>> Making the size of result_buf consistent across each OS and keeping it as large
>> as the Linux BUFSIZ(8192) doesn't really address the core issue.
>>
>> In the event that a user of librte_cmdline creates a custom context with a
>> result structure > 8192 bytes then this problem will occur again, though
>> somewhat unlikely, as the minimum number of the largest type would be 64 x
>> cmdline_fixed_string_t types within a result structure, at its current size.
>>
>> There is no checking of overflow, I would be tempted to add a runtime check in
>> cmdline_parse()/match_inst(), however I would be more comfortable with a build
>> time check for this type of problem.
>>
>> Due to the opaque handling of user defined contexts there is no obvious way to
>> do this at build time.
>>
>> Thoughts?
>
> Indeed, your patch does not address the core issue of the problem,
> altough it's already an improvement to the current situation.
>
> Your issue was already fixed in the latest libcmdline library by
> this patch (which also includes the replacement of BUFSIZ):
> http://git.droids-corp.org/?p=libcmdline.git;a=commitdiff;h=b1d5b169352e57df3fc14c51ffad4b83f3e5613f
>
> I'm pretty sure it won't apply smoothly on the dpdk command line
> library but it can probably be adapted. Ideally, the latest libcmdline
> library should be [cleaned first and] merged in dpdk.org.
Sorry, I had no time to deeply check this. I think your patch can
go in 1.8 as it's still an enhancement compared to the current
situation. We may go back on this later.
Acked-by: Olivier Matz <olivier.matz@6wind.com>
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [dpdk-dev] [PATCH] librte_cmdline: FreeBSD Fix oveflow when size of command result structure is greater than BUFSIZ
2014-12-03 16:05 ` Olivier MATZ
@ 2014-12-03 18:12 ` Thomas Monjalon
2014-12-04 14:01 ` Carew, Alan
0 siblings, 1 reply; 9+ messages in thread
From: Thomas Monjalon @ 2014-12-03 18:12 UTC (permalink / raw)
To: Carew, Alan; +Cc: dev
2014-12-03 17:05, Olivier MATZ:
> Hi,
>
> On 10/27/2014 10:14 AM, Olivier MATZ wrote:
> > Hello Alan,
> >
> > On 10/20/2014 05:26 PM, Carew, Alan wrote:
> >> A comment on my own patch.
> >>
> >> Making the size of result_buf consistent across each OS and keeping it as large
> >> as the Linux BUFSIZ(8192) doesn't really address the core issue.
> >>
> >> In the event that a user of librte_cmdline creates a custom context with a
> >> result structure > 8192 bytes then this problem will occur again, though
> >> somewhat unlikely, as the minimum number of the largest type would be 64 x
> >> cmdline_fixed_string_t types within a result structure, at its current size.
> >>
> >> There is no checking of overflow, I would be tempted to add a runtime check in
> >> cmdline_parse()/match_inst(), however I would be more comfortable with a build
> >> time check for this type of problem.
> >>
> >> Due to the opaque handling of user defined contexts there is no obvious way to
> >> do this at build time.
> >>
> >> Thoughts?
> >
> > Indeed, your patch does not address the core issue of the problem,
> > altough it's already an improvement to the current situation.
> >
> > Your issue was already fixed in the latest libcmdline library by
> > this patch (which also includes the replacement of BUFSIZ):
> > http://git.droids-corp.org/?p=libcmdline.git;a=commitdiff;h=b1d5b169352e57df3fc14c51ffad4b83f3e5613f
> >
> > I'm pretty sure it won't apply smoothly on the dpdk command line
> > library but it can probably be adapted. Ideally, the latest libcmdline
> > library should be [cleaned first and] merged in dpdk.org.
>
> Sorry, I had no time to deeply check this. I think your patch can
> go in 1.8 as it's still an enhancement compared to the current
> situation. We may go back on this later.
>
> Acked-by: Olivier Matz <olivier.matz@6wind.com>
Applied
Thanks
--
Thomas
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [dpdk-dev] [PATCH] librte_cmdline: FreeBSD Fix oveflow when size of command result structure is greater than BUFSIZ
2014-12-03 18:12 ` Thomas Monjalon
@ 2014-12-04 14:01 ` Carew, Alan
2014-12-04 14:15 ` Olivier MATZ
2014-12-04 15:18 ` Thomas Monjalon
0 siblings, 2 replies; 9+ messages in thread
From: Carew, Alan @ 2014-12-04 14:01 UTC (permalink / raw)
To: Thomas Monjalon; +Cc: dev
Hi folks and thanks,
> > Sorry, I had no time to deeply check this. I think your patch can go
> > in 1.8 as it's still an enhancement compared to the current situation.
> > We may go back on this later.
> >
> > Acked-by: Olivier Matz <olivier.matz@6wind.com>
>
> Applied
>
> Thanks
> --
> Thomas
I did post a V2 http://dpdk.org/ml/archives/dev/2014-November/007920.html.
I must have used an incorrect message ID incorrect for "--in-reply-to" though.
Regards,
Alan.
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [dpdk-dev] [PATCH] librte_cmdline: FreeBSD Fix oveflow when size of command result structure is greater than BUFSIZ
2014-12-04 14:01 ` Carew, Alan
@ 2014-12-04 14:15 ` Olivier MATZ
2014-12-04 15:18 ` Thomas Monjalon
1 sibling, 0 replies; 9+ messages in thread
From: Olivier MATZ @ 2014-12-04 14:15 UTC (permalink / raw)
To: Carew, Alan, Thomas Monjalon; +Cc: dev
Hi Alan,
On 12/04/2014 03:01 PM, Carew, Alan wrote:
>>> Sorry, I had no time to deeply check this. I think your patch can go
>>> in 1.8 as it's still an enhancement compared to the current situation.
>>> We may go back on this later.
>>>
>>> Acked-by: Olivier Matz <olivier.matz@6wind.com>
>>
>> Applied
>>
>> Thanks
>> --
>> Thomas
>
> I did post a V2 http://dpdk.org/ml/archives/dev/2014-November/007920.html.
> I must have used an incorrect message ID incorrect for "--in-reply-to" though.
Sorry, I completely missed it. I'll review it ASAP.
Thanks,
Olivier
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [dpdk-dev] [PATCH] librte_cmdline: FreeBSD Fix oveflow when size of command result structure is greater than BUFSIZ
2014-12-04 14:01 ` Carew, Alan
2014-12-04 14:15 ` Olivier MATZ
@ 2014-12-04 15:18 ` Thomas Monjalon
1 sibling, 0 replies; 9+ messages in thread
From: Thomas Monjalon @ 2014-12-04 15:18 UTC (permalink / raw)
To: Carew, Alan; +Cc: dev
2014-12-04 14:01, Carew, Alan:
> I did post a V2 http://dpdk.org/ml/archives/dev/2014-November/007920.html.
> I must have used an incorrect message ID incorrect for "--in-reply-to" though.
Sorry I failed to check similar patches.
I reverted the v1 patch and wait the review to apply the v2.
--
Thomas
^ permalink raw reply [flat|nested] 9+ messages in thread
end of thread, other threads:[~2014-12-04 15:19 UTC | newest]
Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2014-10-20 15:23 [dpdk-dev] [PATCH] librte_cmdline: FreeBSD Fix oveflow when size of command result structure is greater than BUFSIZ Alan Carew
2014-10-20 15:26 ` Carew, Alan
2014-10-20 20:25 ` Thomas Monjalon
2014-10-27 9:14 ` Olivier MATZ
2014-12-03 16:05 ` Olivier MATZ
2014-12-03 18:12 ` Thomas Monjalon
2014-12-04 14:01 ` Carew, Alan
2014-12-04 14:15 ` Olivier MATZ
2014-12-04 15:18 ` Thomas Monjalon
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).