[RFC v2] Add basic syscalls fault injection

[Dmitry V. Levin]

On Wed, May 18, 2016 at 03:59:09AM +0200, Nahim El Atmani wrote:
> Hi,
> 
> On Mon, 16 May 2016 14:46:38 +0300, Dmitry V. Levin wrote:
> > > Since the syscall is directly passed to qual_syscall() everything you used
> > > before is still available (syscall number, name, classes...). The occurrence is
> > > actually an integer
> 
> > Any integer?  Can it be negative or zero?
> 
> Nope, proper checks have to be added. Since this patch is more like a PoC there
> is not much correctness checks as the user is still myself.

No problem, I'd just reviewed it as if it was a final patch.

> > > diff --git a/syscall.c b/syscall.c
> > > index d71ead3..580660c 100644
> > > --- a/syscall.c
> > > +++ b/syscall.c
> > > @@ -266,6 +266,8 @@ enum {
> > >  	MIN_QUALS = MAX_NSYSCALLS > 255 ? MAX_NSYSCALLS : 255
> > >  };
> > >  
> 
> > > +struct_fail fails_vec[MAX_NSYSCALLS];
> 
> > Shouldn't there be not just one vector but SUPPORTED_PERSONALITIES
> > vectors, like qual_vec?
> 
> Yes, I misunderstood why we needed the support for each personalities while
> tracing a program.
> 
> > > +	for (i = 0; i <  nsyscall_vec[current_personality]; ++i)
> > > +		if (sysent_vec[current_personality][i].sys_name
> > > +		    && !strcmp(ss, sysent_vec[current_personality][i].sys_name))
> > > +			break;
> > > +	if (i == nsyscall_vec[current_personality])
> > > +		goto bad_format;
> > > +	qual_syscall(ss, bitflag, not);
> > > +
> > > +	ss = strtok_r(NULL, ":", &saveptr);
> > > +	if (!ss)
> > > +		goto bad_format;
> > > +
> > > +	occ = strtol(ss, &end, 10);
> > > +	if (end == ss || (*end != '\0' && *end != '%') || errno == ERANGE)
> > > +		goto bad_format;
> > > +
> > > +	fails_vec[i].occ = occ;
> > > +	if (*end == '%')
> > > +		fails_vec[i].cnt = -1;
> > > +
> > > +	ss = strtok_r(NULL, ":", &saveptr);
> > > +	if (!ss)
> > > +		goto bad_format;
> > > +
> > > +	if (*ss >= '0' && *ss <= '9') {
> > > +		err = strtol(ss, &end, 10);
> > > +		if (end == ss || *end != '\0' || errno == ERANGE)
> > > +			goto bad_format;
> > > +	}
> > > +	else {
> > > +		/* TODO: support string form. */
> > > +	}
> > > +	fails_vec[i].err = err;
> > > +	fails_vec[i].fail = 0;
> > > +
> > > +	free(ms);
> > > +
> > > +	return 0;
> > > +
> > > +bad_format:
> > > +	free(ms);
> > > +
> > > +	return -1;
> > > +}
> 
> > I suggest a simple change to this procedure: parse the string passed to
> > the function and handle errors, then loop over personalities initializing
> > qual_vec and fails_vec.
> 
> The only reason I see for duplicating code of qual_syscall() and qualify_one()
> is a little speed improvement. Is that what you aim at?

Is there any alternative way to initialize fails_vec for each personality and
syscalls involved?  Maybe qualbits_t could be extended and fails_vec won't
be needed at all?

> > >  	if (!(tcp->qual_flg & QUAL_TRACE)
> > >  	 || (tracing_paths && !pathtrace_match(tcp))
> > > @@ -908,6 +1001,9 @@ trace_syscall_exiting(struct tcb *tcp)
> > >  	update_personality(tcp, tcp->currpers);
> > >  #endif
> > >  	res = (get_regs_error ? -1 : get_syscall_result(tcp));
> > > +
> > > +	if (tcp->qual_flg & QUAL_FAIL && fails_vec[tcp->scno].fail)
> > > +		fail_syscall_exit(tcp);
> > 
> 
> > The check for hide_log_until_execve should go first.
> 
> True, however, and correct me if I'm wrong but one can have both a filtered tcp
> because syscall have not been selected for logging and a failing rule on that
> syscall. That situation can be reflected as:
> strace -e '!write' -e failwith=write:1:EINVAL /bin/ls
> 
> As in « don't output write syscalls since I already know they will fail ».
> 
> If we switch position the goto ret will occur and syscall will *not* be
> discarded.

If hide_log_until_execve is set, all syscalls are invoked by strace itself
and should not be tampered with.

The alternative is to set QUAL_FAIL flag in tcp->qual_flg only when the
decision to fail this particular syscall invocation has been made.


-- 
ldv
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 819 bytes
Desc: not available
URL: <http://lists.strace.io/pipermail/strace-devel/attachments/20160518/e9ad35a9/attachment.bin>