Ticket #337 (Fixed)Fri Feb 22 17:18:22 UTC 2013

inlined memset() of variable on stack could lead to corruption with events (cc 5.69)

Reported by:	Stuart Swales (1481)	Severity:	Normal
Part:	RISC OS: C/C++ toolchain	Release:
Milestone:		Status	Fixed

Details by Stuart Swales (1481):

While debugging PipeDream I had reason to trace into vsnprintf() and was somewhat surprised to see the following code (DecAOF output of fpprintf.o):

0×001028: 706e7376 vsnp : RSBVC r7,r14,r6,ROR r3 0×00102c: 746e6972 rint : STRVCBT r6,[r14],#-0×972 0×001030: 00000066 f… : ANDEQ r0,r0,r6,RRX 0×001034: ff00000c …. : DCI 0xff00000c ; Undefined instruction 0×001038: e1a0c00d …. : MOV r12,r13 0×00103c: e92dd8ef ..-. : STMDB r13!,{r0-r3,r5-r7,r11,r12,r14,pc} 0×001040: e24cb004 ..L. : SUB r11,r12,#4 0×001044: e15d000a ..]. : CMP r13,r10 0×001048: 4bfffbec …K : BLMI __rt_stkovf_split_small 0×00104c: e1b05001 .P.. : MOVS r5,r1 0×001050: e1a06002 .`.. : MOV r6,r2 0×001054: e1a07003 .p.. : MOV r7,r3 0×001058: e24dd028 (.M. : SUB r13,r13,#0×28 0×00105c: e3a01000 …. : MOV r1,#0 0×001060: e3a02000 . .. : MOV r2,#0 0×001064: e3a03000 .0.. : MOV r3,#0 0×001068: e3a0c000 …. : MOV r12,#0 0×00106c: e3a0e000 …. : MOV r14,#0 0×001070: e8ad500e .P.. : STMIA r13!,{r1-r3,r12,r14} 0×001074: e88d500e .P.. : STMIA r13,{r1-r3,r12,r14} 0×001078: e24dd014 ..M. : SUB r13,r13,#0×14 0×00107c: e58d0000 …. : STR r0,[r13] 0×001080: e3a0200a . .. : MOV r2,#0xa 0×001084: 12450001 ..E. : SUBNE r0,r5,#1 0×001088: 03a00000 …. : MOVEQ r0,#0 0×00108c: e58d200c . .. : STR r2,[r13,#0xc] 0×001090: e3a03000 .0.. : MOV r3,#0 0×001094: e58d0008 …. : STR r0,[r13,#8] 0×001098: e52d3004 .0-. : STR r3,[r13,#-4]! 0×00109c: e51f3378 x3.. : LDR r3,0xd2c 0×0010a0: e1a02007 . .. : MOV r2,r7 0×0010a4: e1a01006 …. : MOV r1,r6 0×0010a8: e28d0004 …. : ADD r0,r13,#4 0×0010ac: ebfffbd3 …. : BL __vfprintf

vsnprintf() uses memset() to zero-initialise a FILE structure before filling in some members then calling __vfprintf()

What gives me cause for concern is the method used to zero-initialise:

SUB r13,r13,#0×28 ; FILE structure size
…
STMIA r13!,{r1-r3,r12,r14}
STMIA r13,{r1-r3,r12,r14}
SUB r13,r13,#0×14

This zero-initialises the lower five words, then retracts the stack pointer back over them before zero-initialising the upper five words.

Consider an event being dispatched by the SCL kernel as the STMIA completes and before the stack pointer is advanced back down over the lower five words – they will have been trashed.

I rebuilt the IOMD ROM from today’s source with cc 5.69 and sure enough that’s what fpprintf.c currently compiles to.

This can also be reproduced with the trivial example:

#include <stdio.h>
#include <string.h>

int main(int argc, char * argv[])
{
FILE hack;
memset(&hack, 0, sizeof(hack));
fprintf(&hack, “argc is %d\n”, argc);
return(0);
}