Hi Rob,
On Thu, Sep 01, 2016 at 12:14:02PM +0100, Robert Shearman wrote:
> On 01/09/16 05:34, Salvatore Bonaccorso wrote:
> >Hi,
> >
> >On Tue, Aug 30, 2016 at 10:11:33AM +0100, Robert Shearman wrote:
> >>On Fri, 26 Aug 2016 21:52:11 +0200 Frank Heckenbach
> >><f.heckenb...@fh-soft.de> wrote:
> >>>Package: flex
> >>>Version: 2.5.39-8+deb8u1
> >>>Severity: normal
> >>>
> >>>After this update, I get the following warning when compiling the
> >>>flex generated code with gcc, which I didn't get before:
> >>>
> >>>scan.cpp: In function ??????int yy_get_next_buffer(yyscan_t)??????:
> >>>scan.cpp:758:18: error: comparison between signed and unsigned integer
> >>>expressions [-Werror=sign-compare]
> >>>scan.cpp:1384:3: note: in expansion of macro ??????YY_INPUT??????
> >>>
> >>>Looking at the code:
> >>>
> >>>#define YY_INPUT(buf,result,max_size) \
> >>> if ( YY_CURRENT_BUFFER_LVALUE->yy_is_interactive ) \
> >>> { \
> >>> int c = '*'; \
> >>> size_t n; \
> >>> for ( n = 0; n < max_size && \
> >>>
> >>>Invoked as:
> >>>
> >>>int num_to_read = ...
> >>>YY_INPUT( (&YY_CURRENT_BUFFER_LVALUE->yy_ch_buf[number_to_move]),
> >>> yyg->yy_n_chars, num_to_read );
> >>>
> >>>So indeed an unsigned value (n) is compared with a signed one
> >>>(num_to_read). If this is correct, the warning can be silenced with
> >>>a cast of the appropriate one of them.
> >>
> >>I've run into the same bug and agree with Frank's analysis. I've confirmed
> >>that the following upstream commit fixes it:
> >>
> >>https://github.com/westes/flex/commit/3946924ed5e77420c453bf841603c7278766093a
> >
> >Could you test the following packages:
> >
> >https://people.debian.org/~carnil/tmp/flex/
>
> Thanks for the quick response. Unfortunately, so those packages don't fix
> the warning:
>
> src/npf/npf_scan.c: In function ‘yy_get_next_buffer’:
> src/npf/npf_scan.c:852:18: error: comparison between signed and unsigned
> integer expressions [-Werror=sign-compare]
> for ( n = 0; n < max_size && \
> ^
> src/npf/npf_scan.c:1623:3: note: in expansion of macro ‘YY_INPUT’
> YY_INPUT( (&YY_CURRENT_BUFFER_LVALUE->yy_ch_buf[number_to_move]),
> ^
>
> However, the warning is actually because while the type of n has been fixed,
> num_to_read has reverted to yy_size_t:
>
> #define YY_INPUT(buf,result,max_size) \
> if ( YY_CURRENT_BUFFER_LVALUE->yy_is_interactive ) \
> { \
> int c = '*'; \
> int n; \
> for ( n = 0; n < max_size && \
> (c = getc( yyin )) != EOF && c != '\n'; ++n ) \
>
> ...
>
> yy_size_t num_to_read =
> YY_CURRENT_BUFFER_LVALUE->yy_buf_size -
> number_to_move -
> 1;
>
> So the effect is that 0006-CVE-2016-6354.patch isn't getting applied.
>
> Rebuilding from the sources you also posted, I see that the reason for this
> is that skel.c isn't getting rebuilt, which it should be based on the
> contents of flex.skl. The root cause of this is that skel.c is getting
> patched by quilt at the same time as flex.skl is, so make doesn't see a
> difference in the timestamps.
>
> I've verified that by removing the changes to skel.c from
> 0002-Finish-fixing-the-ia64-buffer-issue.patch and
> 0003-ia64-buffer-fix-Some-more-fixes-for-the-ia64-buffer-.patch results in a
> package being built where the fix works and I don't see the sign comparison
> warning being output from compile of the .c file generated by flex.
You are right, apologies not having spotted it.
> Alternatively, I'm pretty sure that adding the resulting changes to skel.c
> in 0006-CVE-2016-6354.patch would work too.
I decided to actually patch skel.c, since that was the strategy
choosed by Manoj for the previous patches and not to diverge here/to
be consistent.
I uploaded new varaiants of the builds and the corresponding source
package to the same location. Still subject to testing/review before
doing any other steps.
Regards,
Salvatore
