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

[Zhibin Li]

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?

> > +
> > +     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.

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?

Thanks,
Zhibin Li
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.strace.io/pipermail/strace-devel/attachments/20180410/977e52df/attachment.html>