I noticed that tcp->u_args[MAX_ARGS] array is way larger than
I'd expect: for all arches except HPPA it has 32 (!) elements.
I looked at the code and so far I spotted only one abuser of
this fact: sys_sigreturn. On several arches, it saves sigset_t
into tcp->u_args[1...N] on entry and prints it on exit, a-la
memcpy(&tcp->u_arg[1], &sc.oldmask[0], sizeof(sigset_t))
The problem here is that in glibc sigset_t is insanely large:
128 bytes, and using sizeof(sigset_t) in memcpy will overrun
&tcp->u_args[1] even with MAX_ARGS == 32:
On 32 bits, sizeof(tcp->u_args) == 32*4 == 128 bytes!
We may already have a bug there!
I propose to change the code to save NSIG / 8 bytes only.
NSIG can't ever be > 256, and in practice is <= 129,
thus NSIG / 8 is <= 16 bytes == 4 32-bit words,
and MAX_ARGS == 5 should be enough for saving signal masks.
The proposed patch is below.
Alternative solution is to make sys_sigreturn print mask
on entry, not on exit. What is the reson it doesn't do that now?
Please review.
--
vda
diff -d -urpN strace.5/defs.h strace.6/defs.h
--- strace.5/defs.h 2011-08-18 11:57:30.512416447 +0200
+++ strace.6/defs.h 2011-08-18 11:46:56.349540479 +0200
@@ -64,7 +64,7 @@
#define DEFAULT_ACOLUMN 40 /* default alignment column for results
*/
#endif
#ifndef MAX_ARGS
-# ifdef HPPA
+# if defined HPPA || defined X86_64 || defined I386
# define MAX_ARGS 6 /* maximum number of args to a syscall */
# else
/* Way too big. Switch your arch to saner size after you tested that it works
*/
diff -d -urpN strace.5/signal.c strace.6/signal.c
--- strace.5/signal.c 2011-08-18 11:57:57.596606238 +0200
+++ strace.6/signal.c 2011-08-18 11:54:41.032179956 +0200
@@ -262,6 +262,28 @@ static const struct xlat sigprocmaskcmds
#endif
#endif
+/* Note on the size of sigset_t:
+ *
+ * In glibc, sigset_t is an array with space for 1024 bits (!),
+ * even though all arches supported by Linux have only 64 signals
+ * except MIPS, which has 128. IOW, it is 128 bytes long.
+ *
+ * In-kernel sigset_t is sized correctly (it is either 64 or 128 bit long).
+ * However, some old syscall return only 32 lower bits (one word).
+ * Example: sys_sigpending vs sys_rt_sigpending.
+ *
+ * Be aware of this fact when you try to
+ * memcpy(&tcp->u_arg[1], &something, sizeof(sigset_t))
+ * - sizeof(sigset_t) is much bigger than you think,
+ * it may overflow tcp->u_arg[] array, and it may try to copy more data
+ * than is really available in <something>.
+ * Similarly,
+ * umoven(tcp, addr, sizeof(sigset_t), &sigset)
+ * may be a bad idea: it'll try to read much more data than needed
+ * to fetch a sigset_t.
+ * Use (NSIG / 8) as a size instead.
+ */
+
const char *
signame(int sig)
{
@@ -310,11 +332,21 @@ static const char *
sprintsigmask(const char *str, sigset_t *mask, int rt)
/* set might include realtime sigs */
{
+ /* Was [8 * sizeof(sigset_t) * 8], but
+ * glibc sigset_t is huge (1024 bits = 128 *bytes*),
+ * and we were ending up with 8k (!) buffer here.
+ *
+ * No Unix system can have sig > 255
+ * (waitpid API won't be able to indicate death from one)
+ * and sig 0 doesn't exist either.
+ * Therefore max possible no of sigs is 255: 1..255
+ */
+ static char outstr[8 * 255];
+
int i, nsigs;
int maxsigs;
const char *format;
char *s;
- static char outstr[8 * sizeof(sigset_t) * 8];
strcpy(outstr, str);
s = outstr + strlen(outstr);
@@ -1134,7 +1166,7 @@ sys_sigreturn(struct tcb *tcp)
if (umove(tcp, usp + __SIGNAL_FRAMESIZE, &sc) < 0)
return 0;
tcp->u_arg[0] = 1;
- memcpy(&tcp->u_arg[1], &sc.oldmask[0], sizeof(sigset_t));
+ memcpy(&tcp->u_arg[1], &sc.oldmask[0], NSIG / 8);
} else {
tcp->u_rval = tcp->u_error = 0;
if (tcp->u_arg[0] == 0)
------------------------------------------------------------------------------
Get a FREE DOWNLOAD! and learn more about uberSVN rich system,
user administration capabilities and model configuration. Take
the hassle out of deploying and managing Subversion and the
tools developers use with it. http://p.sf.net/sfu/wandisco-d2d-2
_______________________________________________
Strace-devel mailing list
[email protected]
https://lists.sourceforge.net/lists/listinfo/strace-devel
