[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Tinycc-devel] mksh fails to build with bound check
From: |
Kirill Smelkov |
Subject: |
Re: [Tinycc-devel] mksh fails to build with bound check |
Date: |
Sun, 19 Jan 2014 17:03:32 +0400 |
User-agent: |
Mutt/1.5.21 (2010-09-15) |
+grishka
Hi Thomas,
On Wed, Jan 08, 2014 at 03:31:09PM +0800, Thomas Preud'homme wrote:
> Hi Kirill,
>
> it seems you are with Fabrice Bellard the most familiar person with bound
> check code in tcc. Long time ago a bug was reported in Debian about mksh
> failing to work when compiled with tcc and we later found that it was due to
> bound check. Since you did many improvement in the last release, I tried
> again
> a few days ago but unfortunately it still segfaults when compiled with tcc
> and
> bound checking mode on.
>
> Would you mind to take a look? Recent version of mksh have disabled the use
> of
> tcc -b so I use the version 38.3 of mksh (it is the version where the problem
> was found). Once the source downloaded, you just need to go in mksh directory
> in the source and enter CC=tcc CFLAGS=-g sh Build.sh -r. Then try running
> mksh
> with the most basic command: ./mksh -c true and look at the segfault. This
> procedure is described at [0].
>
> [0] http://bugs.debian.org/cgi-bin/bugreport.cgi?bug=537622
>
> Anyway, even if you can't or don't want to take a look at this bug I'd like
> to
> thank you for improving the bound check code in tcc. It's one of its key
> feature that makes this project cool and that people like.
>
> Best regards,
>
> Thomas
Sorry for being silent for so long. I've started looking into the
problem today, but before I've ever got to mksh, I've found one new bug
wrt bcheck which is now fixed. Could you please see, does it maybe fixes
mksh? If not, I'm willing to continue digging, only it will be very
slow, as my time is very tight this days...
P.S. Grishka, I think the tests "cleanup" you've done in
d5f4df09 "tests: cleanup", where you removed test1b,test2b and test3b
tests is not right - gaining `tcc -b` being able to compile itself was
hard, and tests were there to assure it will stay that working way
without regressions.
Kirill
---- 8< ----
From: Kirill Smelkov <address@hidden>
Date: Sun, 19 Jan 2014 16:35:20 +0400
Subject: [PATCH] tccrun: Mark argv area as valid for bcheck
On my x86_64 box in i386 mode with address space randomization turned off,
I've observed the following:
tests$ ../tcc -B.. -b -run boundtest.c 1
Runtime error: dereferencing invalid pointer
boundtest.c:222: at 0x808da73 main()
With diagnostic patch (like in efd9d92b "lib/bcheck: Don't assume heap
goes right after bss") and bcheck traces for __bound_new_region,
__bound_ptr_indir, etc... here is how the program run looks like:
>>> TCC
etext: 0x8067ed8
edata: 0x807321d
end: 0x807d95c
brk: 0x807e000
stack: 0xffffd0b4
&errno: 0xf7dbd688
mark_invalid 0xfff80000 - (nil)
mark_invalid 0x80fa000 - 0x100fa000
new 808fdb0 808ff40 101 101 fd0 ff0
new 808ff44 808ff48 101 101 ff0 ff0
new 808ff49 8090049 101 101 ff0 1000
new 808fd20 808fd29 101 101 fd0 fd0
new 808fd2c 808fd6c 101 101 fd0 fd0
new 808fd6d 808fda0 101 101 fd0 fd0
E: __bound_ptr_indir4(0xffffd184, 0x4)
Runtime error: dereferencing invalid pointer
boundtest.c:222: at 0x808ea83 main()
So we are accessing something on stack, above stack entry for compiled
main. Investigating with gdb shows that this is argv:
tests$ gdb ../tcc
Reading symbols from /home/kirr/src/tools/tinycc/tcc...done.
(gdb) set args -B.. -b -run boundtest.c 1
(gdb) r
Starting program: /home/kirr/src/tools/tinycc/tests/../tcc -B.. -b -run
boundtest.c 1
warning: Could not load shared library symbols for linux-gate.so.1.
Do you need "set solib-search-path" or "set sysroot"?
>>> TCC
etext: 0x8067ed8
edata: 0x807321d
end: 0x807d95c
brk: 0x807e000
stack: 0xffffd074
&errno: 0xf7dbd688
mark_invalid 0xfff80000 - (nil)
mark_invalid 0x80fa000 - 0x100fa000
new 808fdb0 808ff40 101 101 fd0 ff0
new 808ff44 808ff48 101 101 ff0 ff0
new 808ff49 8090049 101 101 ff0 1000
new 808fd20 808fd29 101 101 fd0 fd0
new 808fd2c 808fd6c 101 101 fd0 fd0
new 808fd6d 808fda0 101 101 fd0 fd0
E: __bound_ptr_indir4(0xffffd144, 0x4)
Program received signal SIGSEGV, Segmentation fault.
0x0808ea83 in ?? ()
(gdb) bt
#0 0x0808ea83 in ?? ()
#1 0x080639b3 in tcc_run (address@hidden, address@hidden, address@hidden)
at tccrun.c:132
#2 0x080492b0 in main (argc=6, argv=0xffffd134) at tcc.c:346
(gdb) f 1
#1 0x080639b3 in tcc_run (address@hidden, address@hidden, address@hidden)
at tccrun.c:132
132 ret = (*prog_main)(argc, argv);
132 ret = (*prog_main)(argc, argv);
(gdb) p argv
$1 = (char **) 0xffffd144
So before running compiled program, mark argv as valid region and we are
done - now the test passes.
P.S. maybe it would be better to just mark the whole vector kernel passes to
program (argv, env, auxv, etc...) as valid all at once...
---
tccrun.c | 17 +++++++++++++++++
1 file changed, 17 insertions(+)
diff --git a/tccrun.c b/tccrun.c
index b07ab0f..55fb3d8 100644
--- a/tccrun.c
+++ b/tccrun.c
@@ -110,13 +110,30 @@ LIBTCCAPI int tcc_run(TCCState *s1, int argc, char **argv)
if (s1->do_bounds_check) {
void (*bound_init)(void);
void (*bound_exit)(void);
+ void (*bound_new_region)(void *p, unsigned long size);
+ int (*bound_delete_region)(void *p);
+ int i;
+
/* set error function */
rt_bound_error_msg = tcc_get_symbol_err(s1, "__bound_error_msg");
/* XXX: use .init section so that it also work in binary ? */
bound_init = tcc_get_symbol_err(s1, "__bound_init");
bound_exit = tcc_get_symbol_err(s1, "__bound_exit");
+ bound_new_region = tcc_get_symbol_err(s1, "__bound_new_region");
+ bound_delete_region = tcc_get_symbol_err(s1, "__bound_delete_region");
bound_init();
+ /* mark argv area as valid */
+ bound_new_region(argv, argc*sizeof(argv[0]));
+ for (i=0; i<argc; ++i)
+ bound_new_region(argv[i], strlen(argv[i]));
+
ret = (*prog_main)(argc, argv);
+
+ /* unmark argv area */
+ for (i=0; i<argc; ++i)
+ bound_delete_region(argv[i]);
+ bound_delete_region(argv);
+
bound_exit();
} else
#endif
--
1.9.rc0.143.g6fd479e