* PATCH: Fix buffer overflow in gas
@ 2006-05-01 18:40 H. J. Lu
2006-05-02 9:48 ` Nick Clifton
0 siblings, 1 reply; 7+ messages in thread
From: H. J. Lu @ 2006-05-01 18:40 UTC (permalink / raw)
To: binutils
There are some potential buffer overflows in gas. 8byte isn't enough
to hold a negative byte. This patch fixes them. Also we should use
snprintf instead of sprintf.
H.J.
---
2006-05-01 H.J. Lu <hongjiu.lu@intel.com>
* config/tc-i386.c (output_invalid_buf): Change size to 16.
* config/tc-tic30.c (output_invalid_buf): Likewise.
* config/tc-i386.c (output_invalid): Use snprintf instead of
sprintf.
* config/tc-ia64.c (declare_register_set): Likewise.
(emit_one_bundle): Likewise.
(check_dependencies): Likewise.
* config/tc-tic30.c (output_invalid): Likewise.
--- gas/config/tc-i386.c.buf 2006-04-25 14:35:46.000000000 -0700
+++ gas/config/tc-i386.c 2006-05-01 11:13:22.000000000 -0700
@@ -5251,16 +5251,18 @@ md_atof (type, litP, sizeP)
return 0;
}
\f
-static char output_invalid_buf[8];
+static char output_invalid_buf[16];
static char *
output_invalid (c)
int c;
{
if (ISPRINT (c))
- sprintf (output_invalid_buf, "'%c'", c);
+ snprintf (output_invalid_buf, sizeof (output_invalid_buf),
+ "'%c'", c);
else
- sprintf (output_invalid_buf, "(0x%x)", (unsigned) c);
+ snprintf (output_invalid_buf, sizeof (output_invalid_buf),
+ "(0x%x)", (unsigned) c);
return output_invalid_buf;
}
--- gas/config/tc-ia64.c.buf 2006-04-25 14:35:46.000000000 -0700
+++ gas/config/tc-ia64.c 2006-05-01 11:26:49.000000000 -0700
@@ -5634,7 +5634,7 @@ declare_register_set (prefix, num_regs,
for (i = 0; i < num_regs; ++i)
{
- sprintf (name, "%s%u", prefix, i);
+ snprintf (name, sizeof (name), "%s%u", prefix, i);
declare_register (name, base_regnum + i);
}
}
@@ -6971,7 +6971,8 @@ emit_one_bundle ()
else
as_fatal ("emit_one_bundle: unexpected dynamic op");
- sprintf (mnemonic, "%s.%c", idesc->name, "?imbfxx"[insn_unit]);
+ snprintf (mnemonic, sizeof (mnemonic), "%s.%c",
+ idesc->name, "?imbfxx"[insn_unit]);
opnd1 = idesc->operands[0];
opnd2 = idesc->operands[1];
ia64_free_opcode (idesc);
@@ -10544,12 +10545,15 @@ check_dependencies (idesc)
int certain = (matchtype == 1 && CURR_SLOT.qp_regno == 0);
if (path != 0)
- sprintf (pathmsg, " when entry is at label '%s'",
+ snprintf (pathmsg, sizeof (pathmsg),
+ " when entry is at label '%s'",
md.entry_labels[path - 1]);
if (matchtype == 1 && rs->index >= 0)
- sprintf (indexmsg, ", specific resource number is %d",
+ snprintf (indexmsg, sizeof (indexmsg),
+ ", specific resource number is %d",
rs->index);
- sprintf (msg, "Use of '%s' %s %s dependency '%s' (%s)%s%s",
+ snprintf (msg, sizeof (msg),
+ "Use of '%s' %s %s dependency '%s' (%s)%s%s",
idesc->name,
(certain ? "violates" : "may violate"),
dv_mode[dep->mode], dep->name,
--- gas/config/tc-tic30.c.buf 2005-08-15 07:50:53.000000000 -0700
+++ gas/config/tc-tic30.c 2006-05-01 11:13:53.000000000 -0700
@@ -273,15 +273,17 @@ struct tic30_insn
struct tic30_insn insn;
static int found_parallel_insn;
-static char output_invalid_buf[8];
+static char output_invalid_buf[16];
static char *
output_invalid (char c)
{
if (ISPRINT (c))
- sprintf (output_invalid_buf, "'%c'", c);
+ snprintf (output_invalid_buf, sizeof (output_invalid_buf),
+ "'%c'", c);
else
- sprintf (output_invalid_buf, "(0x%x)", (unsigned) c);
+ snprintf (output_invalid_buf, sizeof (output_invalid_buf),
+ "(0x%x)", (unsigned) c);
return output_invalid_buf;
}
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: PATCH: Fix buffer overflow in gas
2006-05-01 18:40 PATCH: Fix buffer overflow in gas H. J. Lu
@ 2006-05-02 9:48 ` Nick Clifton
2006-05-02 13:31 ` H. J. Lu
0 siblings, 1 reply; 7+ messages in thread
From: Nick Clifton @ 2006-05-02 9:48 UTC (permalink / raw)
To: H. J. Lu; +Cc: binutils
Hi H. J.
> There are some potential buffer overflows in gas. 8byte isn't enough
> to hold a negative byte. This patch fixes them. Also we should use
> snprintf instead of sprintf.
Did you test this patch ? if so, please could you say how.
> 2006-05-01 H.J. Lu <hongjiu.lu@intel.com>
>
> * config/tc-i386.c (output_invalid_buf): Change size to 16.
> * config/tc-tic30.c (output_invalid_buf): Likewise.
>
> * config/tc-i386.c (output_invalid): Use snprintf instead of
> sprintf.
> * config/tc-ia64.c (declare_register_set): Likewise.
> (emit_one_bundle): Likewise.
> (check_dependencies): Likewise.
> * config/tc-tic30.c (output_invalid): Likewise.
Assuming that you have tested the targets involved and that there were
no regressions then this patch is approved.
Cheers
Nick
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: PATCH: Fix buffer overflow in gas
2006-05-02 9:48 ` Nick Clifton
@ 2006-05-02 13:31 ` H. J. Lu
2006-05-02 14:20 ` H. J. Lu
0 siblings, 1 reply; 7+ messages in thread
From: H. J. Lu @ 2006-05-02 13:31 UTC (permalink / raw)
To: Nick Clifton; +Cc: binutils
On Tue, May 02, 2006 at 10:47:27AM +0100, Nick Clifton wrote:
> Hi H. J.
>
> >There are some potential buffer overflows in gas. 8byte isn't enough
> >to hold a negative byte. This patch fixes them. Also we should use
> >snprintf instead of sprintf.
>
> Did you test this patch ? if so, please could you say how.
You can put some none ascii char in assembly code.
>
> >2006-05-01 H.J. Lu <hongjiu.lu@intel.com>
> >
> > * config/tc-i386.c (output_invalid_buf): Change size to 16.
> > * config/tc-tic30.c (output_invalid_buf): Likewise.
> >
> > * config/tc-i386.c (output_invalid): Use snprintf instead of
> > sprintf.
> > * config/tc-ia64.c (declare_register_set): Likewise.
> > (emit_one_bundle): Likewise.
> > (check_dependencies): Likewise.
> > * config/tc-tic30.c (output_invalid): Likewise.
>
> Assuming that you have tested the targets involved and that there were
> no regressions then this patch is approved.
Done.
H.J.
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: PATCH: Fix buffer overflow in gas
2006-05-02 13:31 ` H. J. Lu
@ 2006-05-02 14:20 ` H. J. Lu
2006-05-02 14:35 ` Jan Beulich
0 siblings, 1 reply; 7+ messages in thread
From: H. J. Lu @ 2006-05-02 14:20 UTC (permalink / raw)
To: Nick Clifton; +Cc: binutils, jbeulich
On Tue, May 02, 2006 at 06:31:30AM -0700, H. J. Lu wrote:
> On Tue, May 02, 2006 at 10:47:27AM +0100, Nick Clifton wrote:
> > Hi H. J.
> >
> > >There are some potential buffer overflows in gas. 8byte isn't enough
> > >to hold a negative byte. This patch fixes them. Also we should use
> > >snprintf instead of sprintf.
> >
> > Did you test this patch ? if so, please could you say how.
>
> You can put some none ascii char in assembly code.
>
> >
> > >2006-05-01 H.J. Lu <hongjiu.lu@intel.com>
> > >
> > > * config/tc-i386.c (output_invalid_buf): Change size to 16.
> > > * config/tc-tic30.c (output_invalid_buf): Likewise.
> > >
> > > * config/tc-i386.c (output_invalid): Use snprintf instead of
> > > sprintf.
> > > * config/tc-ia64.c (declare_register_set): Likewise.
> > > (emit_one_bundle): Likewise.
> > > (check_dependencies): Likewise.
> > > * config/tc-tic30.c (output_invalid): Likewise.
> >
> > Assuming that you have tested the targets involved and that there were
> > no regressions then this patch is approved.
>
> Done.
>
Jan suggested we should cast int to unsigned char.
foo.s:20: Error: invalid character (0xd6) in mnemonic
is better than
foo.s:20: Error: invalid character (0xfffffffd6) in mnemonic
I will check it in.
H.J.
---
2006-05-02 H.J. Lu <hongjiu.lu@intel.com>
Jan Beulich <jbeulich@novell.com>
* config/tc-i386.c (output_invalid_buf): Change size for
unsigned char.
* config/tc-tic30.c (output_invalid_buf): Likewise.
* config/tc-i386.c (output_invalid): Cast none-ascii char to
unsigned char.
* config/tc-tic30.c (output_invalid): Likewise.
--- gas/config/tc-i386.c.buf 2006-05-02 06:50:00.000000000 -0700
+++ gas/config/tc-i386.c 2006-05-02 07:12:03.000000000 -0700
@@ -5251,7 +5251,7 @@ md_atof (type, litP, sizeP)
return 0;
}
\f
-static char output_invalid_buf[16];
+static char output_invalid_buf[sizeof (unsigned char) * 2 + 6];
static char *
output_invalid (c)
@@ -5262,7 +5262,7 @@ output_invalid (c)
"'%c'", c);
else
snprintf (output_invalid_buf, sizeof (output_invalid_buf),
- "(0x%x)", (unsigned) c);
+ "(0x%x)", (unsigned char) c);
return output_invalid_buf;
}
--- gas/config/tc-tic30.c.buf 2006-05-02 06:50:00.000000000 -0700
+++ gas/config/tc-tic30.c 2006-05-02 07:12:11.000000000 -0700
@@ -273,7 +273,7 @@ struct tic30_insn
struct tic30_insn insn;
static int found_parallel_insn;
-static char output_invalid_buf[16];
+static char output_invalid_buf[sizeof (unsigned char) * 2 + 6];
static char *
output_invalid (char c)
@@ -283,7 +283,7 @@ output_invalid (char c)
"'%c'", c);
else
snprintf (output_invalid_buf, sizeof (output_invalid_buf),
- "(0x%x)", (unsigned) c);
+ "(0x%x)", (unsigned char) c);
return output_invalid_buf;
}
2006-05-02 H.J. Lu <hongjiu.lu@intel.com>
* config/tc-i386.c (output_invalid_buf): Change size for
unsigned char.
* config/tc-tic30.c (output_invalid_buf): Likewise.
* config/tc-i386.c (output_invalid): Cast none-ascii char to
unsigned char.
* config/tc-tic30.c (output_invalid): Likewise.
--- gas/config/tc-i386.c.buf 2006-05-02 06:50:00.000000000 -0700
+++ gas/config/tc-i386.c 2006-05-02 07:12:03.000000000 -0700
@@ -5251,7 +5251,7 @@ md_atof (type, litP, sizeP)
return 0;
}
\f
-static char output_invalid_buf[16];
+static char output_invalid_buf[sizeof (unsigned char) * 2 + 6];
static char *
output_invalid (c)
@@ -5262,7 +5262,7 @@ output_invalid (c)
"'%c'", c);
else
snprintf (output_invalid_buf, sizeof (output_invalid_buf),
- "(0x%x)", (unsigned) c);
+ "(0x%x)", (unsigned char) c);
return output_invalid_buf;
}
--- gas/config/tc-tic30.c.buf 2006-05-02 06:50:00.000000000 -0700
+++ gas/config/tc-tic30.c 2006-05-02 07:12:11.000000000 -0700
@@ -273,7 +273,7 @@ struct tic30_insn
struct tic30_insn insn;
static int found_parallel_insn;
-static char output_invalid_buf[16];
+static char output_invalid_buf[sizeof (unsigned char) * 2 + 6];
static char *
output_invalid (char c)
@@ -283,7 +283,7 @@ output_invalid (char c)
"'%c'", c);
else
snprintf (output_invalid_buf, sizeof (output_invalid_buf),
- "(0x%x)", (unsigned) c);
+ "(0x%x)", (unsigned char) c);
return output_invalid_buf;
}
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: PATCH: Fix buffer overflow in gas
2006-05-02 14:20 ` H. J. Lu
@ 2006-05-02 14:35 ` Jan Beulich
2006-05-02 14:58 ` H. J. Lu
0 siblings, 1 reply; 7+ messages in thread
From: Jan Beulich @ 2006-05-02 14:35 UTC (permalink / raw)
To: H. Lu; +Cc: Nick Clifton, binutils
>+static char output_invalid_buf[sizeof (unsigned char) * 2 + 6];
I'm sorry to say that this, but regardless of actual width of 'char' or 'unsigned char' the above declaration still
always produces an 8-unit array. What you mean is making room for as many hex digits as an 'unsigned char' can be
converted to. Without knowing CHAR_BIT (or equivalent) I cannot see how you would be able to derive that. Unfortunately,
previous work on binutils has shown that one apparently shouldn't make assumptions about the availability of limits.h
(despite this having been a standard header for at least 15 years), and hence CHAR_BIT is not generally usable here.
Where needed, I (and apparently others) generally made the assumption that if limits.h isn't available, then the machine
is supposed to be a 8-bit-bytes one (and bad luck to those perhaps hypothetical machines that aren't and that don't
provide the header).
Jan
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: PATCH: Fix buffer overflow in gas
2006-05-02 14:35 ` Jan Beulich
@ 2006-05-02 14:58 ` H. J. Lu
2006-05-02 16:53 ` Andreas Schwab
0 siblings, 1 reply; 7+ messages in thread
From: H. J. Lu @ 2006-05-02 14:58 UTC (permalink / raw)
To: Jan Beulich; +Cc: Nick Clifton, binutils
On Tue, May 02, 2006 at 04:36:19PM +0200, Jan Beulich wrote:
> >+static char output_invalid_buf[sizeof (unsigned char) * 2 + 6];
>
> I'm sorry to say that this, but regardless of actual width of 'char' or 'unsigned char' the above declaration still
> always produces an 8-unit array. What you mean is making room for as many hex digits as an 'unsigned char' can be
> converted to. Without knowing CHAR_BIT (or equivalent) I cannot see how you would be able to derive that. Unfortunately,
> previous work on binutils has shown that one apparently shouldn't make assumptions about the availability of limits.h
> (despite this having been a standard header for at least 15 years), and hence CHAR_BIT is not generally usable here.
> Where needed, I (and apparently others) generally made the assumption that if limits.h isn't available, then the machine
> is supposed to be a 8-bit-bytes one (and bad luck to those perhaps hypothetical machines that aren't and that don't
> provide the header).
I don't want to get too complicated here. We won't have buffer overflow
anymore no matter what CHAR_BIT is. Feel free to improve it.
H.J.
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: PATCH: Fix buffer overflow in gas
2006-05-02 14:58 ` H. J. Lu
@ 2006-05-02 16:53 ` Andreas Schwab
0 siblings, 0 replies; 7+ messages in thread
From: Andreas Schwab @ 2006-05-02 16:53 UTC (permalink / raw)
To: H. J. Lu; +Cc: Jan Beulich, Nick Clifton, binutils
"H. J. Lu" <hjl@lucon.org> writes:
> I don't want to get too complicated here. We won't have buffer overflow
> anymore no matter what CHAR_BIT is.
If you have more than 8 bits in a char you need more than two characters
to represent it as a hex number. On the other hand, whether we get to
process more than an octet at a time in this case remains to be seen.
Andreas.
--
Andreas Schwab, SuSE Labs, schwab@suse.de
SuSE Linux Products GmbH, MaxfeldstraÃe 5, 90409 Nürnberg, Germany
PGP key fingerprint = 58CA 54C7 6D53 942B 1756 01D3 44D5 214B 8276 4ED5
"And now for something completely different."
^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2006-05-02 16:53 UTC | newest]
Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2006-05-01 18:40 PATCH: Fix buffer overflow in gas H. J. Lu
2006-05-02 9:48 ` Nick Clifton
2006-05-02 13:31 ` H. J. Lu
2006-05-02 14:20 ` H. J. Lu
2006-05-02 14:35 ` Jan Beulich
2006-05-02 14:58 ` H. J. Lu
2006-05-02 16:53 ` Andreas Schwab
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 read-only IMAP folder(s) and NNTP newsgroup(s).