[PATCH v4 02/10] tests/inject-nf.test: replace getpid with geteuid

[Dmitry V. Levin]

On Tue, Jun 16, 2020 at 01:18:20PM +0200, Ákos Uzonyi wrote:
> On Tue, 16 Jun 2020 at 00:22, Dmitry V. Levin <ldv at altlinux.org> wrote:
> > On Mon, Jun 15, 2020 at 11:00:31PM +0200, Ákos Uzonyi wrote:
> > > On Mon, 15 Jun 2020 at 21:31, Dmitry V. Levin <ldv at altlinux.org> wrote:
> > > > On Mon, Jun 15, 2020 at 08:29:33PM +0200, Ákos Uzonyi wrote:
> > > > > On Mon, 15 Jun 2020 at 17:45, Dmitry V. Levin <ldv at altlinux.org> wrote:
> > > > > > On Mon, Jun 15, 2020 at 03:01:43PM +0200, Ákos Uzonyi wrote:
> > > > > > > On Mon, 15 Jun 2020 at 01:37, Dmitry V. Levin <ldv at altlinux.org> wrote:
> > > > > > > > On Sat, Jun 13, 2020 at 06:18:29PM +0200, Ákos Uzonyi wrote:
> > > > > > > > > Since we treat PIDs as signed integers, large values (>=2^31) will
> > > > > > > > > cause overflow when we use printpid.
> > > > > > > > > UIDs are treated as unsigned integers, so geteuid is a good alternative.
> > > > > > > > > (getuid would be problematic, as it does not exists on alpha).
> > > > > > > >
> > > > > > > > No, this approach doesn't work because
> > > > > > > > $ cat /proc/sys/kernel/overflowuid
> > > > > > > > 65534
> > > > > > > > $ ./inject-nf
> > > > > > > > expected 0x25bd0, got rval=0xfffe err=0
> > > > > > > >
> > > > > > > > See also tests/overflowuid.c file.
> > > > > > >
> > > > > > > Sorry, I don't understand why overflowuid is significant here.
> > > > > >
> > > > > > geteuid syscall is affected by overflowuid on such architectures
> > > > > > as x86 where geteuid32 syscall exists.
> > > > >
> > > > > If I understand correctly, the problem happens only when we run
> > > > > inject-nf without injection. But why do we want to do that? Would it
> > > > > be OK to remove the "run_prog" line from tests/inject-nf.test?
> > > >
> > > > We use run_prog to make sure the program works properly without strace.
> > > > In case of geteuid it doesn't because geteuid is not universal enough.
> > >
> > > What about replacing geteuid() call with INVOKE_SC(err) in expected value?
> >
> > This would result to comparison of INVOKE_SC(err) with itself.
> 
> And why is that a problem?

It's pointless to compare INVOKE_SC(err) with itself because it will
always match.

> I mean, what do we want to achieve by
> running inject-nf without arguments? Is it really about testing
> whether a libc syscall wrapper (which we do not even use in the real
> test) gives the same result as a raw syscall? Seems pointless to me.

We want to make sure that INVOKE_SC(err) meets our expectations,
otherwise it's unsafe to use it for testing syscall tampering.

> I would suggest either removing "run_prog", or replacing geteuid()
> with INVOKE_SC(err). I think this latter case is perfectly enough for
> testing whether the program works correctly without strace. But if
> none of these is acceptable, please tell me the rationale behind
> "run_prog", otherwise I can't fix this test.

I think the test can be changed e.g. to use geteuid32 on those architectures
where geteuid is subject for /proc/sys/kernel/overflowuid wrapping.


-- 
ldv