Re: [PATCH] Avoid potential NULL deref in scripts/genksyms/lex.l

2007-06-24 Thread Jesper Juhl

On 25/06/07, Andrew Morton <[EMAIL PROTECTED]> wrote:

On Mon, 25 Jun 2007 00:02:03 +0200 "Jesper Juhl" <[EMAIL PROTECTED]> wrote:

> > > +  if (!file || !e)
> > > + exit(1);
> > >*e = '\0';
> > >cur_filename = memcpy(xmalloc(e-file+1), file, e-file+1);
> > >cur_line = atoi(yytext+2);
> >
> > I don't think the bug which you're fixing can occur:
> >
> > ^#[ \t]+{INT}[ \t]+\"[^\"\n]+\".*\n return FILENAME;
> >
> > has anyone reported crashes in there?
> >
>
> It may indeed not be possible. Found by inspection, not by any actual
> observed crashes.
> But does it really hurt to be defensive here? In case it can somehow
> be caused to fail, with my patch it'll fail a bit nicer :) It's not
> like it's at all speed critical code that will be hurt by that extra
> 'if'...

We can only get NULL pointers here if the regexp is wrong or if there's
a bug in the regexp parser.  Getting a coredump at the failure site is
much better behaviour than mysteriously exiting.


Fair enough. Just ignore the patch then :-)

--
Jesper Juhl <[EMAIL PROTECTED]>
Don't top-post  http://www.catb.org/~esr/jargon/html/T/top-post.html
Plain text mails only, please  http://www.expita.com/nomime.html
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH] Avoid potential NULL deref in scripts/genksyms/lex.l

2007-06-24 Thread Andrew Morton
On Mon, 25 Jun 2007 00:02:03 +0200 "Jesper Juhl" <[EMAIL PROTECTED]> wrote:

> > > +  if (!file || !e)
> > > + exit(1);
> > >*e = '\0';
> > >cur_filename = memcpy(xmalloc(e-file+1), file, e-file+1);
> > >cur_line = atoi(yytext+2);
> >
> > I don't think the bug which you're fixing can occur:
> >
> > ^#[ \t]+{INT}[ \t]+\"[^\"\n]+\".*\n return FILENAME;
> >
> > has anyone reported crashes in there?
> >
> 
> It may indeed not be possible. Found by inspection, not by any actual
> observed crashes.
> But does it really hurt to be defensive here? In case it can somehow
> be caused to fail, with my patch it'll fail a bit nicer :) It's not
> like it's at all speed critical code that will be hurt by that extra
> 'if'...

We can only get NULL pointers here if the regexp is wrong or if there's
a bug in the regexp parser.  Getting a coredump at the failure site is
much better behaviour than mysteriously exiting.
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH] Avoid potential NULL deref in scripts/genksyms/lex.l

2007-06-24 Thread Jesper Juhl

On 24/06/07, Andrew Morton <[EMAIL PROTECTED]> wrote:

On Sun, 24 Jun 2007 23:40:03 +0200 Jesper Juhl <[EMAIL PROTECTED]> wrote:

> strchr() returns NULL in case the string is not found and if that
> happens we risk dereferencing a NULL pointer. It never hurts to
> check for that condition and exit normally with an error rather
> than crashing.
>
> (no, the indentation is not according to CodingStyle, it's simply
>  following whatever else is in that file)
>
>
> Signed-off-by: Jesper Juhl <[EMAIL PROTECTED]>
> ---
>
>  scripts/genksyms/lex.l|2 ++
>  1 files changed, 2 insertions(+), 0 deletions(-)
>
> diff --git a/scripts/genksyms/lex.l b/scripts/genksyms/lex.l
> index 5e544a0..28edc0c 100644
> --- a/scripts/genksyms/lex.l
> +++ b/scripts/genksyms/lex.l
> @@ -154,6 +154,8 @@ repeat:
>
>file = strchr(yytext, '\"')+1;
>e = strchr(file, '\"');

If `file' can be null we'd have oopsed here.

> +  if (!file || !e)
> + exit(1);
>*e = '\0';
>cur_filename = memcpy(xmalloc(e-file+1), file, e-file+1);
>cur_line = atoi(yytext+2);

I don't think the bug which you're fixing can occur:

^#[ \t]+{INT}[ \t]+\"[^\"\n]+\".*\n return FILENAME;

has anyone reported crashes in there?



It may indeed not be possible. Found by inspection, not by any actual
observed crashes.
But does it really hurt to be defensive here? In case it can somehow
be caused to fail, with my patch it'll fail a bit nicer :) It's not
like it's at all speed critical code that will be hurt by that extra
'if'...

--
Jesper Juhl <[EMAIL PROTECTED]>
Don't top-post  http://www.catb.org/~esr/jargon/html/T/top-post.html
Plain text mails only, please  http://www.expita.com/nomime.html
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH] Avoid potential NULL deref in scripts/genksyms/lex.l

2007-06-24 Thread Andrew Morton
On Sun, 24 Jun 2007 23:40:03 +0200 Jesper Juhl <[EMAIL PROTECTED]> wrote:

> strchr() returns NULL in case the string is not found and if that 
> happens we risk dereferencing a NULL pointer. It never hurts to 
> check for that condition and exit normally with an error rather 
> than crashing.
> 
> (no, the indentation is not according to CodingStyle, it's simply 
>  following whatever else is in that file)
> 
> 
> Signed-off-by: Jesper Juhl <[EMAIL PROTECTED]>
> ---
> 
>  scripts/genksyms/lex.l|2 ++
>  1 files changed, 2 insertions(+), 0 deletions(-)
> 
> diff --git a/scripts/genksyms/lex.l b/scripts/genksyms/lex.l
> index 5e544a0..28edc0c 100644
> --- a/scripts/genksyms/lex.l
> +++ b/scripts/genksyms/lex.l
> @@ -154,6 +154,8 @@ repeat:
>  
>file = strchr(yytext, '\"')+1;
>e = strchr(file, '\"');

If `file' can be null we'd have oopsed here.

> +  if (!file || !e)
> + exit(1);
>*e = '\0';
>cur_filename = memcpy(xmalloc(e-file+1), file, e-file+1);
>cur_line = atoi(yytext+2);

I don't think the bug which you're fixing can occur:

^#[ \t]+{INT}[ \t]+\"[^\"\n]+\".*\n return FILENAME;

has anyone reported crashes in there?
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH] Avoid potential NULL deref in scripts/genksyms/lex.l

2007-06-24 Thread Andrew Morton
On Sun, 24 Jun 2007 23:40:03 +0200 Jesper Juhl [EMAIL PROTECTED] wrote:

 strchr() returns NULL in case the string is not found and if that 
 happens we risk dereferencing a NULL pointer. It never hurts to 
 check for that condition and exit normally with an error rather 
 than crashing.
 
 (no, the indentation is not according to CodingStyle, it's simply 
  following whatever else is in that file)
 
 
 Signed-off-by: Jesper Juhl [EMAIL PROTECTED]
 ---
 
  scripts/genksyms/lex.l|2 ++
  1 files changed, 2 insertions(+), 0 deletions(-)
 
 diff --git a/scripts/genksyms/lex.l b/scripts/genksyms/lex.l
 index 5e544a0..28edc0c 100644
 --- a/scripts/genksyms/lex.l
 +++ b/scripts/genksyms/lex.l
 @@ -154,6 +154,8 @@ repeat:
  
file = strchr(yytext, '\')+1;
e = strchr(file, '\');

If `file' can be null we'd have oopsed here.

 +  if (!file || !e)
 + exit(1);
*e = '\0';
cur_filename = memcpy(xmalloc(e-file+1), file, e-file+1);
cur_line = atoi(yytext+2);

I don't think the bug which you're fixing can occur:

^#[ \t]+{INT}[ \t]+\[^\\n]+\.*\n return FILENAME;

has anyone reported crashes in there?
-
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH] Avoid potential NULL deref in scripts/genksyms/lex.l

2007-06-24 Thread Jesper Juhl

On 24/06/07, Andrew Morton [EMAIL PROTECTED] wrote:

On Sun, 24 Jun 2007 23:40:03 +0200 Jesper Juhl [EMAIL PROTECTED] wrote:

 strchr() returns NULL in case the string is not found and if that
 happens we risk dereferencing a NULL pointer. It never hurts to
 check for that condition and exit normally with an error rather
 than crashing.

 (no, the indentation is not according to CodingStyle, it's simply
  following whatever else is in that file)


 Signed-off-by: Jesper Juhl [EMAIL PROTECTED]
 ---

  scripts/genksyms/lex.l|2 ++
  1 files changed, 2 insertions(+), 0 deletions(-)

 diff --git a/scripts/genksyms/lex.l b/scripts/genksyms/lex.l
 index 5e544a0..28edc0c 100644
 --- a/scripts/genksyms/lex.l
 +++ b/scripts/genksyms/lex.l
 @@ -154,6 +154,8 @@ repeat:

file = strchr(yytext, '\')+1;
e = strchr(file, '\');

If `file' can be null we'd have oopsed here.

 +  if (!file || !e)
 + exit(1);
*e = '\0';
cur_filename = memcpy(xmalloc(e-file+1), file, e-file+1);
cur_line = atoi(yytext+2);

I don't think the bug which you're fixing can occur:

^#[ \t]+{INT}[ \t]+\[^\\n]+\.*\n return FILENAME;

has anyone reported crashes in there?



It may indeed not be possible. Found by inspection, not by any actual
observed crashes.
But does it really hurt to be defensive here? In case it can somehow
be caused to fail, with my patch it'll fail a bit nicer :) It's not
like it's at all speed critical code that will be hurt by that extra
'if'...

--
Jesper Juhl [EMAIL PROTECTED]
Don't top-post  http://www.catb.org/~esr/jargon/html/T/top-post.html
Plain text mails only, please  http://www.expita.com/nomime.html
-
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH] Avoid potential NULL deref in scripts/genksyms/lex.l

2007-06-24 Thread Andrew Morton
On Mon, 25 Jun 2007 00:02:03 +0200 Jesper Juhl [EMAIL PROTECTED] wrote:

   +  if (!file || !e)
   + exit(1);
  *e = '\0';
  cur_filename = memcpy(xmalloc(e-file+1), file, e-file+1);
  cur_line = atoi(yytext+2);
 
  I don't think the bug which you're fixing can occur:
 
  ^#[ \t]+{INT}[ \t]+\[^\\n]+\.*\n return FILENAME;
 
  has anyone reported crashes in there?
 
 
 It may indeed not be possible. Found by inspection, not by any actual
 observed crashes.
 But does it really hurt to be defensive here? In case it can somehow
 be caused to fail, with my patch it'll fail a bit nicer :) It's not
 like it's at all speed critical code that will be hurt by that extra
 'if'...

We can only get NULL pointers here if the regexp is wrong or if there's
a bug in the regexp parser.  Getting a coredump at the failure site is
much better behaviour than mysteriously exiting.
-
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH] Avoid potential NULL deref in scripts/genksyms/lex.l

2007-06-24 Thread Jesper Juhl

On 25/06/07, Andrew Morton [EMAIL PROTECTED] wrote:

On Mon, 25 Jun 2007 00:02:03 +0200 Jesper Juhl [EMAIL PROTECTED] wrote:

   +  if (!file || !e)
   + exit(1);
  *e = '\0';
  cur_filename = memcpy(xmalloc(e-file+1), file, e-file+1);
  cur_line = atoi(yytext+2);
 
  I don't think the bug which you're fixing can occur:
 
  ^#[ \t]+{INT}[ \t]+\[^\\n]+\.*\n return FILENAME;
 
  has anyone reported crashes in there?
 

 It may indeed not be possible. Found by inspection, not by any actual
 observed crashes.
 But does it really hurt to be defensive here? In case it can somehow
 be caused to fail, with my patch it'll fail a bit nicer :) It's not
 like it's at all speed critical code that will be hurt by that extra
 'if'...

We can only get NULL pointers here if the regexp is wrong or if there's
a bug in the regexp parser.  Getting a coredump at the failure site is
much better behaviour than mysteriously exiting.


Fair enough. Just ignore the patch then :-)

--
Jesper Juhl [EMAIL PROTECTED]
Don't top-post  http://www.catb.org/~esr/jargon/html/T/top-post.html
Plain text mails only, please  http://www.expita.com/nomime.html
-
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/