| Message ID | 1538504783-26793-1-git-send-email-pkusunyifeng@gmail.com |
|---|---|
| State | Superseded |
| Headers | show |
| Series | [ovs-dev] lex: Fix stack underflow bug and improve loop | expand |
Sorry just realized my patches are missing the [ovs-dev] in the subjects. Resend v2. On Tue, Oct 2, 2018 at 11:26 AM Yifeng Sun <pkusunyifeng@gmail.com> wrote: > In previous code, if hexit == 0, then the boundary for 'out' is > not checked. This patch fixes it and also takes the checking out > of loop to improve loop's performance. > > Reported-at: https://bugs.chromium.org/p/oss-fuzz/issues/detail?id=10710 > Signed-off-by > <https://bugs.chromium.org/p/oss-fuzz/issues/detail?id=10710Signed-off-by>: > Yifeng Sun <pkusunyifeng@gmail.com> > --- > ovn/lib/lex.c | 11 ++++++----- > 1 file changed, 6 insertions(+), 5 deletions(-) > > diff --git a/ovn/lib/lex.c b/ovn/lib/lex.c > index 0514950de6bf..269267730526 100644 > --- a/ovn/lib/lex.c > +++ b/ovn/lib/lex.c > @@ -327,17 +327,18 @@ lex_parse_hex_integer(const char *start, size_t len, > struct lex_token *token) > const char *in = start + (len - 1); > uint8_t *out = token->value.u8 + (sizeof token->value.u8 - 1); > > + if ((len - 1) / 2 >= sizeof token->value.u8) { > + lex_error(token, "Hexadecimal constant requires more than " > + "%"PRIuSIZE" bits.", 8 * sizeof token->value.u8); > + return; > + } > + > for (int i = 0; i < len; i++) { > int hexit = hexit_value(in[-i]); > if (hexit < 0) { > lex_error(token, "Invalid syntax in hexadecimal constant."); > return; > } > - if (hexit && i / 2 >= sizeof token->value.u8) { > - lex_error(token, "Hexadecimal constant requires more than " > - "%"PRIuSIZE" bits.", 8 * sizeof token->value.u8); > - return; > - } > out[-(i / 2)] |= i % 2 ? hexit << 4 : hexit; > } > token->format = LEX_F_HEXADECIMAL; > -- > 2.7.4 > >
On Tue, Oct 02, 2018 at 11:26:23AM -0700, Yifeng Sun wrote: > In previous code, if hexit == 0, then the boundary for 'out' is > not checked. This patch fixes it and also takes the checking out > of loop to improve loop's performance. > > Reported-at: https://bugs.chromium.org/p/oss-fuzz/issues/detail?id=10710 > Signed-off-by: Yifeng Sun <pkusunyifeng@gmail.com> Thanks for the fix! I think that my goal here was to ignore any number of leading zeros, more like this: diff --git a/ovn/lib/lex.c b/ovn/lib/lex.c index 0514950de6bf..a5237091a46b 100644 --- a/ovn/lib/lex.c +++ b/ovn/lib/lex.c @@ -332,13 +332,14 @@ lex_parse_hex_integer(const char *start, size_t len, struct lex_token *token) if (hexit < 0) { lex_error(token, "Invalid syntax in hexadecimal constant."); return; + } else if (hexit) { + if (i / 2 >= sizeof token->value.u8) { + lex_error(token, "Hexadecimal constant requires more than " + "%"PRIuSIZE" bits.", 8 * sizeof token->value.u8); + return; + } + out[-(i / 2)] |= i % 2 ? hexit << 4 : hexit; } - if (hexit && i / 2 >= sizeof token->value.u8) { - lex_error(token, "Hexadecimal constant requires more than " - "%"PRIuSIZE" bits.", 8 * sizeof token->value.u8); - return; - } - out[-(i / 2)] |= i % 2 ? hexit << 4 : hexit; } token->format = LEX_F_HEXADECIMAL; }
Thanks Ben for the review, I sent a v3. On Tue, Oct 2, 2018 at 11:41 AM Ben Pfaff <blp@ovn.org> wrote: > On Tue, Oct 02, 2018 at 11:26:23AM -0700, Yifeng Sun wrote: > > In previous code, if hexit == 0, then the boundary for 'out' is > > not checked. This patch fixes it and also takes the checking out > > of loop to improve loop's performance. > > > > Reported-at: https://bugs.chromium.org/p/oss-fuzz/issues/detail?id=10710 > > Signed-off-by: Yifeng Sun <pkusunyifeng@gmail.com> > > Thanks for the fix! > > I think that my goal here was to ignore any number of leading zeros, > more like this: > > diff --git a/ovn/lib/lex.c b/ovn/lib/lex.c > index 0514950de6bf..a5237091a46b 100644 > --- a/ovn/lib/lex.c > +++ b/ovn/lib/lex.c > @@ -332,13 +332,14 @@ lex_parse_hex_integer(const char *start, size_t len, > struct lex_token *token) > if (hexit < 0) { > lex_error(token, "Invalid syntax in hexadecimal constant."); > return; > + } else if (hexit) { > + if (i / 2 >= sizeof token->value.u8) { > + lex_error(token, "Hexadecimal constant requires more than > " > + "%"PRIuSIZE" bits.", 8 * sizeof > token->value.u8); > + return; > + } > + out[-(i / 2)] |= i % 2 ? hexit << 4 : hexit; > } > - if (hexit && i / 2 >= sizeof token->value.u8) { > - lex_error(token, "Hexadecimal constant requires more than " > - "%"PRIuSIZE" bits.", 8 * sizeof token->value.u8); > - return; > - } > - out[-(i / 2)] |= i % 2 ? hexit << 4 : hexit; > } > token->format = LEX_F_HEXADECIMAL; > } >
diff --git a/ovn/lib/lex.c b/ovn/lib/lex.c index 0514950de6bf..269267730526 100644 --- a/ovn/lib/lex.c +++ b/ovn/lib/lex.c @@ -327,17 +327,18 @@ lex_parse_hex_integer(const char *start, size_t len, struct lex_token *token) const char *in = start + (len - 1); uint8_t *out = token->value.u8 + (sizeof token->value.u8 - 1); + if ((len - 1) / 2 >= sizeof token->value.u8) { + lex_error(token, "Hexadecimal constant requires more than " + "%"PRIuSIZE" bits.", 8 * sizeof token->value.u8); + return; + } + for (int i = 0; i < len; i++) { int hexit = hexit_value(in[-i]); if (hexit < 0) { lex_error(token, "Invalid syntax in hexadecimal constant."); return; } - if (hexit && i / 2 >= sizeof token->value.u8) { - lex_error(token, "Hexadecimal constant requires more than " - "%"PRIuSIZE" bits.", 8 * sizeof token->value.u8); - return; - } out[-(i / 2)] |= i % 2 ? hexit << 4 : hexit; } token->format = LEX_F_HEXADECIMAL;
In previous code, if hexit == 0, then the boundary for 'out' is not checked. This patch fixes it and also takes the checking out of loop to improve loop's performance. Reported-at: https://bugs.chromium.org/p/oss-fuzz/issues/detail?id=10710 Signed-off-by: Yifeng Sun <pkusunyifeng@gmail.com> --- ovn/lib/lex.c | 11 ++++++----- 1 file changed, 6 insertions(+), 5 deletions(-)