On Wed, Aug 31, 2011 at 12:01:06PM +0000, Denys Vlasenko wrote:
> commit 1d46ba57a8ab16b353b531f2bbefe2ad7f354ca9
> Author: Denys Vlasenko <[email protected]>
> Date:   Wed Aug 31 14:00:02 2011 +0200
> 
>     Make out-of-memory handling more uniform
>     
>     This fixes one real bug in dumpstr().
>     
>     * defs.h: Declare die_out_of_memory().
>     * strace.c (die_out_of_memory): New function.
>     (strace_popen): If allocation fails, call die_out_of_memory().
>     (main): Likewise.
>     (expand_tcbtab): Likewise.
>     (rebuild_pollv): Likewise.
>     * count.c (count_syscall): Likewise.
>     (call_summary_pers): Likewise.
>     * desc.c (decode_select): Likewise.
>     * file.c (sys_getdents): Likewise.
>     (sys_getdents64): Likewise.
>     (sys_getdirentries): Likewise.
>     * pathtrace.c (pathtrace_match): Likewise.
>     * syscall.c (qualify): Likewise.
>     * util.c (printstr): Likewise.
>     (dumpiov): Likewise.
>     (dumpstr): Likewise.
>     (fixvfork): Likewise.
>     * mem.c (sys_mincore): Don't check free() parameter for NULL.

There are no need to hurry with changes like this.
Not every ENOMEM is really fatal.

> --- a/desc.c
> +++ b/desc.c
> @@ -497,9 +497,9 @@ decode_select(struct tcb *tcp, long *args, enum bitness_t 
> bitness)
>       long arg;
>  
>       if (entering(tcp)) {
> -             fds = (fd_set *) malloc(fdsize);
> -             if (fds == NULL)
> -                     fprintf(stderr, "out of memory\n");
> +             fds = malloc(fdsize);
> +             if (!fds)
> +                     die_out_of_memory();

For example, this memory allocation error is certainly not fatal.
fdsize value is based on untrusted input and therefore can be quite large.

For example,

$ cat select.c 
#include <sys/select.h>
int main(void)
{
        static struct timeval t;
        return select(2147483640, 0, 0, 0, &t) < 0;
}

It will cause strace to call malloc(268435456), and the old code would
handle this:

$ (ulimit -v 262144; strace -eselect ./select)
select(out of memory
2147483640, NULL, NULL, NULL, {0, 0}) = 0 (Timeout)

Not as nice as it could be, but better than what the new code does.

Please check other parts of this change, I expect some of them also have
to be revisited.


-- 
ldv

Attachment: pgpLj5PNNJila.pgp
Description: PGP signature

------------------------------------------------------------------------------
Special Offer -- Download ArcSight Logger for FREE!
Finally, a world-class log management solution at an even better 
price-free! And you'll get a free "Love Thy Logs" t-shirt when you
download Logger. Secure your free ArcSight Logger TODAY!
http://p.sf.net/sfu/arcsisghtdev2dev
_______________________________________________
Strace-devel mailing list
[email protected]
https://lists.sourceforge.net/lists/listinfo/strace-devel

Reply via email to