[PATCH v5] tests/fcntl.c: add test for print_f_owner_ex

[Dmitry V. Levin]

On Tue, Apr 10, 2018 at 12:21:35AM +0800, Zhibin Li wrote:
> On Sun, Apr 8, 2018 at 12:50 AM, Dmitry V. Levin <ldv at altlinux.org> wrote:
> > On Sun, Apr 08, 2018 at 12:10:14AM +0800, Zhibin Li wrote:
> > > *tests/fcntl.c (test_f_owner_ex_type_pid)
> > > (test_f_owner_ex_umove, test_f_owner_ex_printaddr)
> > > (test_f_owner_ex): New functions.
> > > (main): Use test_f_owner_ex.
> > > ---
> > >  tests/fcntl.c | 45 +++++++++++++++++++++++++++++++++++++++++++++
> > >  1 file changed, 45 insertions(+)
> > >
> > > diff --git a/tests/fcntl.c b/tests/fcntl.c
> > > index 4f62ca2a..e9db81ef 100644
> > > --- a/tests/fcntl.c
> > > +++ b/tests/fcntl.c
> > > @@ -69,12 +69,57 @@ test_flock64(void)
> > >  #endif
> > >  }
> > >
> > > +static long
> > > +test_f_owner_ex_type_pid(const int cmd, const char *const cmd_name,
> > > +                      const int type, const char *const type_name,
> > > +                      const pid_t pid)
> > > +{
> > > +     struct f_owner_ex fo = { .type = type, .pid = pid };
> >
> > What if you used TAIL_ALLOC_OBJECT_CONST_PTR to create an object
> > that cannot be read beyond its end?
> >
> >  Why using TAIL_ALLOC_OBJECT_CONST_PTR here? The purpose seems not
> so explicit to me. Is it about optimization or something else?

As described, we use TAIL_ALLOC_OBJECT_CONST_PTR to "allocate an object
of the specified type at the end of a mapped memory region".

This way we can test decoding of unfetchable objects.

> > > +
> > > +     long rc = invoke_test_syscall(cmd, &fo);
> > > +     printf("%s(0, %s, {type=%s, pid=%d}) = %s\n",
> > > +            TEST_SYSCALL_STR, cmd_name, type_name, pid, sprintrc(rc));
> > > +     return rc;
> > > +}
> > > +
> > > +static void
> > > +test_f_owner_ex_umove(const int type, const char *const type_name,
> > pid_t pid)
> > > +{
> > > +     long rc = test_f_owner_ex_type_pid(ARG_STR(F_SETOWN_EX),
> > > +                                        type, type_name, pid);
> > > +     if (!rc)
> > > +             test_f_owner_ex_type_pid(ARG_STR(F_GETOWN_EX),
> > > +                                      type, type_name, pid);
> > > +}
> > > +
> > > +static void
> > > +test_f_owner_ex_printaddr(const int cmd, const char *const cmd_name)
> > > +{
> > > +     long rc = invoke_test_syscall(cmd, (void *const)0x7ffde503d9e8);
> > > +     printf("%s(0, %s, 0x7ffde503d9e8) = %s\n",
> > > +            TEST_SYSCALL_STR, cmd_name, sprintrc(rc));
> > > +}
> >
> > Why this magic constant?  What guarantees that it doesn't reference
> > to a valid mapped memory?  How is it expected to work on 32-bit systems?
> >
> > I was thinking about using NULL because in some cases the output of
> printaddr is NULL.
> But when the umoven function can't fetch the specific data from the given
> address,
>  printaddr should print the memory address instead of NULL. I'm a little
> confused about
> what is expected. Just an invalid mapped memory is OK? As Dmitry mentioned
> above, what if
> I use TAIL_ALLOC_OBJECT_CONST_PTR here to get a piece of memory at the end
> and then
> plus a constant(e.g. TAIL_ALLOC_OBJECT_CONST_PTR(struct f_owner_ex, fo); fo
> + 1)
> so that it will not be a valid address to fetch the data(in this case, it's
> struct f_owner_ex).
> I'm not so sure about the feasibility of my idea and I'd be thankful if
> anyone can answer
> my questions.

Yes, this is exactly what we do with objects allocated at the end of
a mapped memory region:
- first, we pass a pointer to this valid object and check whether
the corresponding decoder tries to access memory beyond the end
of the object;
- second, we pass a pointer to an object that cannot be fetched, e.g. the
  previous pointer + constant, to check decoding of an unfetchable object.

Magic numbers like yours 0x7ffde503d9e8 don't work well, they might happen
to be valid addresses.

> By the way, in the file strace/fcntl.c, I found some assignments like const
> unsigned int cmd = tcp->u_arg[1];
> But I noticed the type of u_arg in struct tcb is kernel_ulong_t. Does this
> need type casting? If not,
> why is that?

There is no need for an explicit cast because of implicit type conversion.


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