re: Problem with syscall_disestablish() - PR kern/50430

[Paul Goyette <paul%vps1.whooppee.com@localhost>]

On Tue, 17 Nov 2015, Paul Goyette wrote:

> > there's a fairly significant problem with this implementation.
> >
> > you're only catching callers who use end up going through sy_call()
> > and that's not the majority.  mostly they're called directly from
> > MD code.  so to fix that, you have to update every platform syscall
> > handler.
>
> Ah, I see.  I'll have to investigate further.

It seems that the sy_call() mechanism is used for all "modular"
syscalls.  As you correctly point out, it is not used for all
syscalls.

(Quoting from further down)

> The problem I'm attempting to address is when syscall #1 is blocked,
> some signal gets delivered to the lwp, and the signal handler calls some
> other syscall #2.  In that case, the single l_sysent gets overwritten
> and we now have no record that syscall #1 is still active.

So, the problem seems to become one of much smaller magnitude that it
was at first glance.  It's only a problem if both syscalls #1 and #2
are entered via sy_call(), rather than for any arbitrary syscall #2.

> > it also slows down normal operation adding branches and data access
> > to the call path of every system call.  and bloats struct lwp and
> > more.

As above, it only affects system calls that go through sy_call().  It
does, however, bloat every struct lwp.

It would be possible to transfer that bloat out of struct lwp and into
struct proc.  But that would require an extra dereference in sy_call()
for l->l_proc->.  I'm not sure whether I want memory bloat or execution
bloat.  :)

===

There is one problem I discovered with my current implementation...

The l_scdepth member is currently just incremented and decremented in
sy_call(), and the mask is checked in syscall_disestablish() only when
the depth is greater than 1.  In the case of syscall#1 --> sig-handler
--> syscall#2 --> return-from-#2 --> return-from-sig-handler and back
to the blockage in execution of syscall#2, we would decrement the
l_scdepth member back to 1.  Syscall#1's l_sysent would already have
been overwritten, but we won't check the mask because the depth is 1.

I'm proposing to solve this by replacing the

	l->l_scdepth--;

with

	l->l_scdepth &= ~1;

As a result, the l_scdepth member will have three states: 0, 1, and 
moer-than-1-at-some-point-in-time (values of 2 or more will never be
decremented below 2).  So now the check in syscall_disestablish()
becomes

	if (any lwp has ever had more than 1 active syscall, and that
	    lwp has ever executed a syscall that belongs to the set), {
		disallow the disestablish()
	}


+------------------+--------------------------+-------------------------+
| Paul Goyette     | PGP Key fingerprint:     | E-mail addresses:       |
| (Retired)        | FA29 0E3B 35AF E8AE 6651 | paul at whooppee.com    |
| Kernel Developer | 0786 F758 55DE 53BA 7731 | pgoyette at netbsd.org  |
+------------------+--------------------------+-------------------------+