Re: [LTP] [PATCH] lib/tst_uid_gid.c: fix checking return value errors about getpwuid_r/getgrgid_r

[chrubis@suse.cz]

Hi!
> Hmm, as far as I can see the the error for bin.gr_gid is EPERM, while
> invalid user test expect EINVAL, so something must be different.
> 
> And it looks like you can really get EINVAL in the setregid() in
> kernel/sys.c. And that it depends on namespace mappings. See
> kernel/user_namespace.c make_kgid() that calls map_id_down(), although I
> haven't yet figured out exact conditions yet.

And here is the rest of the story.

Unless process modified it's user namespace the get_current_ns() will
return pointer to init_user_ns that is defined in kernel/user.c. This
user namespace maps gids and uids 1:1 in the interval 0 - 4294967294.
Because if I understand the code right the map_id_down checks if id is
in interval between first and first + count - 1 and the count is set to
4294967295. The 4294967295 on the other hand equals to (u32)-1 which is
not included in the mapping but this is the value used for error exit so
it maps uids and gids 1:1 by default.


I've looked closely at the setregid() syscall code and I'm pretty sure
that you can't get EINVAL unless you played with namespaces. Looking
closely at the setregid02 testcase the test is wrong see closely these
lines:


                                if (TEST_ERRNO == test_data[i].exp_errno) {
                                        tst_resm(TPASS, "setregid(%d, %d) "
                                                 "failed as expected.",
                                                 *test_data[i].real_gid,
                                                 *test_data[i].eff_gid);
                                } else if (TEST_ERRNO == test_data[0].exp_errno) {
                                        tst_resm(TPASS, "setregid(%d, %d) "
                                                 "failed as expected.",
                                                 *test_data[i].real_gid,
                                                 *test_data[i].eff_gid);
                                } else {
                                        tst_resm(TFAIL, "setregid(%d, %d) "
                                                 "failed (%d) but did not set the "
                                                 "expected errno (%d).",
                                                 *test_data[i].real_gid,
                                                 *test_data[i].eff_gid,
                                                 TEST_ERRNO,
                                                 test_data[i].exp_errno);
                                }


The test allows the call to faile with either it's expected errno or with the
expected errno of the first testcase. This looks like ugly workaround for some
kernel changes.

The git log says:

    Correct testcase return on RHEL 3 & 4 2.6.13-rc6-mm1

So the best solution would be to remove this workaround and the two testcases
as well.


The setresuid() is the same, it cannot return EINVAL unless namespaces were
involved namespaces were modified. I would go for removing the redundant
testcases there as well.

Uff, that's it, to sum it up, I would go for removing the testcase that employs
GET_INVALID* got rid of the tst_ functions as well. Sorry that I didn't get it
right the first time.


PS: The codepath looks like it can return ENOMEM. I wonder if we can get it
    if we play with process ulimits.

-- 
Cyril Hrubis
chrubis@suse.cz

------------------------------------------------------------------------------
Put Bad Developers to Shame
Dominate Development with Jenkins Continuous Integration
Continuously Automate Build, Test & Deployment 
Start a new project now. Try Jenkins in the cloud.
http://p.sf.net/sfu/13600_Cloudbees
_______________________________________________
Ltp-list mailing list
Ltp-list@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/ltp-list