* [9fans] possible buffer overflow in devcons.c?
@ 2014-06-18 17:29 Yoann Padioleau
2014-06-18 17:34 ` erik quanstrom
2014-06-18 17:34 ` andrey mirtchovski
0 siblings, 2 replies; 7+ messages in thread
From: Yoann Padioleau @ 2014-06-18 17:29 UTC (permalink / raw)
To: Fans of the OS Plan 9 from Bell Labs
Hi,
In devcons.c there is
/*
* Put character, possibly a rune, into read queue at interrupt time.
* Called at interrupt time to process a character.
*/
int
kbdputc(Queue*, int ch)
{
int i, n;
char buf[3]; <----- enough?
Rune r;
char *next;
if(kbd.ir == nil)
return 0; /* in case we're not inited yet */
ilock(&kbd.lockputc); /* just a mutex */
r = ch;
n = runetochar(buf, &r);
for(i = 0; i < n; i++){
next = kbd.iw+1;
if(next >= kbd.ie)
next = kbd.istage;
if(next == kbd.ir)
break;
*kbd.iw = buf[i];
kbd.iw = next;
}
iunlock(&kbd.lockputc);
return 0;
}
But is the buf[3] enough? UTFMAX is 4 so we could possibly overflow no?
Shouldn't it be buf[UTFMAX] ?
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [9fans] possible buffer overflow in devcons.c?
2014-06-18 17:29 [9fans] possible buffer overflow in devcons.c? Yoann Padioleau
@ 2014-06-18 17:34 ` erik quanstrom
2014-06-18 17:34 ` andrey mirtchovski
1 sibling, 0 replies; 7+ messages in thread
From: erik quanstrom @ 2014-06-18 17:34 UTC (permalink / raw)
To: 9fans
> But is the buf[3] enough? UTFMAX is 4 so we could possibly overflow no?
> Shouldn't it be buf[UTFMAX] ?
yes, it should. i thought i'd caught all of those.
- erik
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [9fans] possible buffer overflow in devcons.c?
2014-06-18 17:29 [9fans] possible buffer overflow in devcons.c? Yoann Padioleau
2014-06-18 17:34 ` erik quanstrom
@ 2014-06-18 17:34 ` andrey mirtchovski
2014-06-18 17:36 ` erik quanstrom
1 sibling, 1 reply; 7+ messages in thread
From: andrey mirtchovski @ 2014-06-18 17:34 UTC (permalink / raw)
To: Fans of the OS Plan 9 from Bell Labs
used to be 3 :)
"UTFmax, defined as 3 in <libc.h>, is the maximum number of bytes
required to represent a rune."
http://man.cat-v.org/plan_9/2/rune
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [9fans] possible buffer overflow in devcons.c?
2014-06-18 17:34 ` andrey mirtchovski
@ 2014-06-18 17:36 ` erik quanstrom
2014-06-18 18:13 ` Yoann Padioleau
0 siblings, 1 reply; 7+ messages in thread
From: erik quanstrom @ 2014-06-18 17:36 UTC (permalink / raw)
To: 9fans
On Wed Jun 18 13:36:09 EDT 2014, mirtchovski@gmail.com wrote:
> used to be 3 :)
>
> "UTFmax, defined as 3 in <libc.h>, is the maximum number of bytes
> required to represent a rune."
which is exactly why this should have been caught.
this one's my fault.
- erik
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [9fans] possible buffer overflow in devcons.c?
2014-06-18 17:36 ` erik quanstrom
@ 2014-06-18 18:13 ` Yoann Padioleau
2014-06-18 18:18 ` erik quanstrom
0 siblings, 1 reply; 7+ messages in thread
From: Yoann Padioleau @ 2014-06-18 18:13 UTC (permalink / raw)
To: Fans of the OS Plan 9 from Bell Labs
Well, hard to grep for '3' in a big codebase. They should have used UTFMAX in the first place.
I see though that currently in libc.h I have
enum
{
UTFmax = 4, /* maximum bytes per rune */
Runesync = 0x80, /* cannot represent part of a UTF sequence (<) */
Runeself = 0x80, /* rune and UTF sequences are the same (<) */
Runeerror = 0xFFFD, /* decoding error in UTF */
Runemax = 0x10FFFF, /* 21-bit rune */
Runemask = 0x1FFFFF, /* bits used by runes (see grep) */
};
so Runemax seems to indicate we never produce rune using more than 3 bytes no?
So maybe buf[3] is safe?
On Jun 18, 2014, at 10:36 AM, erik quanstrom <quanstro@quanstro.net> wrote:
> On Wed Jun 18 13:36:09 EDT 2014, mirtchovski@gmail.com wrote:
>> used to be 3 :)
>>
>> "UTFmax, defined as 3 in <libc.h>, is the maximum number of bytes
>> required to represent a rune."
>
> which is exactly why this should have been caught.
> this one's my fault.
>
> - erik
>
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [9fans] possible buffer overflow in devcons.c?
2014-06-18 18:13 ` Yoann Padioleau
@ 2014-06-18 18:18 ` erik quanstrom
2014-06-18 18:44 ` erik quanstrom
0 siblings, 1 reply; 7+ messages in thread
From: erik quanstrom @ 2014-06-18 18:18 UTC (permalink / raw)
To: 9fans
On Wed Jun 18 14:15:14 EDT 2014, pad@fb.com wrote:
> Well, hard to grep for '3' in a big codebase. They should have used UTFMAX in the first place.
>
it's not hard. i did this once. i think i did something like this
g 'char[ ]+[a-z]+\[(3|4|3\+1)\]' /sys/src
- erik
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [9fans] possible buffer overflow in devcons.c?
2014-06-18 18:18 ` erik quanstrom
@ 2014-06-18 18:44 ` erik quanstrom
0 siblings, 0 replies; 7+ messages in thread
From: erik quanstrom @ 2014-06-18 18:44 UTC (permalink / raw)
To: 9fans
On Wed Jun 18 14:20:02 EDT 2014, quanstro@quanstro.net wrote:
> On Wed Jun 18 14:15:14 EDT 2014, pad@fb.com wrote:
> > Well, hard to grep for '3' in a big codebase. They should have used UTFMAX in the first place.
> >
>
> it's not hard. i did this once. i think i did something like this
>
> g 'char[ ]+[a-z]+\[(3|4|3\+1)\]' /sys/src
er, better to have
g '( | |^)char[ ]+[a-z]+\[(3|3 *\+ *1)\]' /sys/src
it looks like 9atom is clean. i haven't checked sources. but that
reminds me i have to kill that private copy of devcons in omap.
- erik
^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2014-06-18 18:44 UTC | newest]
Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2014-06-18 17:29 [9fans] possible buffer overflow in devcons.c? Yoann Padioleau
2014-06-18 17:34 ` erik quanstrom
2014-06-18 17:34 ` andrey mirtchovski
2014-06-18 17:36 ` erik quanstrom
2014-06-18 18:13 ` Yoann Padioleau
2014-06-18 18:18 ` erik quanstrom
2014-06-18 18:44 ` erik quanstrom
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).