* [LTP] [PATCH] lib/tst_uid_gid.c: fix checking return value errors about getpwuid_r/getgrgid_r
@ 2014-02-20 2:10 Zeng Linggang
2014-02-20 7:53 ` Stanislav Kholmanskikh
` (2 more replies)
0 siblings, 3 replies; 12+ messages in thread
From: Zeng Linggang @ 2014-02-20 2:10 UTC (permalink / raw)
To: ltp-list
When we call getgrgid_r() to verify whether the corresponding gid is
existent, if not, getgrgid_r() will return 0 or ENOENT or ESRCH or
EBADF or EPERM...
But tst_get_unused_gid() only check return value 0, this is not complete.
For example, if the gid is not existent, getgrgid_r() in RHEL7 beta will
return ENOENT, which will cause tst_get_unused_gid failed incorrectly.
This is the same for getpwuid_r().
We should make tst_get_unused_uid/tst_get_unused_gid check all the possible
return value.
Signed-off-by: Zeng Linggang <zenglg.jy@cn.fujitsu.com>
---
lib/tst_uid_gid.c | 31 +++++++++++++++++++++++++++----
1 file changed, 27 insertions(+), 4 deletions(-)
diff --git a/lib/tst_uid_gid.c b/lib/tst_uid_gid.c
index 3ba1ad6..a835c38 100644
--- a/lib/tst_uid_gid.c
+++ b/lib/tst_uid_gid.c
@@ -16,6 +16,7 @@
* Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA
*/
+#include <errno.h>
#include <grp.h>
#include <limits.h>
#include <pwd.h>
@@ -44,10 +45,21 @@ uid_t tst_get_unused_uid(void)
s = getpwuid_r(uid, &pwd, buf, bufsize, &result);
if (result == NULL) {
free(buf);
- if (s == 0)
+ /*
+ * When the given name or gid was not found, getgrgid_r
+ * may return 0 or ENOENT or ESRCH or EBADF or EPERM
+ * or ...
+ */
+ switch (s) {
+ case 0:
+ case ENOENT:
+ case ESRCH:
+ case EBADF:
+ case EPERM:
return uid;
- else
+ default:
return -1;
+ }
}
}
@@ -76,10 +88,21 @@ gid_t tst_get_unused_gid(void)
s = getgrgid_r(gid, &grp, buf, bufsize, &result);
if (result == NULL) {
free(buf);
- if (s == 0)
+ /*
+ * When the given name or gid was not found, getgrgid_r
+ * may return 0 or ENOENT or ESRCH or EBADF or EPERM
+ * or ...
+ */
+ switch (s) {
+ case 0:
+ case ENOENT:
+ case ESRCH:
+ case EBADF:
+ case EPERM:
return gid;
- else
+ default:
return -1;
+ }
}
}
--
1.8.4.2
------------------------------------------------------------------------------
Managing the Performance of Cloud-Based Applications
Take advantage of what the Cloud has to offer - Avoid Common Pitfalls.
Read the Whitepaper.
http://pubads.g.doubleclick.net/gampad/clk?id=121054471&iu=/4140/ostg.clktrk
_______________________________________________
Ltp-list mailing list
Ltp-list@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/ltp-list
^ permalink raw reply related [flat|nested] 12+ messages in thread
* Re: [LTP] [PATCH] lib/tst_uid_gid.c: fix checking return value errors about getpwuid_r/getgrgid_r
2014-02-20 2:10 [LTP] [PATCH] lib/tst_uid_gid.c: fix checking return value errors about getpwuid_r/getgrgid_r Zeng Linggang
@ 2014-02-20 7:53 ` Stanislav Kholmanskikh
2014-03-06 11:34 ` Zeng Linggang
2014-03-13 8:33 ` Zeng Linggang
[not found] ` <1396944482.2100.1.camel@G08JYZSD130126>
2 siblings, 1 reply; 12+ messages in thread
From: Stanislav Kholmanskikh @ 2014-02-20 7:53 UTC (permalink / raw)
To: Zeng Linggang, ltp-list
Hi!
On 02/20/2014 06:10 AM, Zeng Linggang wrote:
> When we call getgrgid_r() to verify whether the corresponding gid is
> existent, if not, getgrgid_r() will return 0 or ENOENT or ESRCH or
> EBADF or EPERM...
>
> But tst_get_unused_gid() only check return value 0, this is not complete.
> For example, if the gid is not existent, getgrgid_r() in RHEL7 beta will
> return ENOENT, which will cause tst_get_unused_gid failed incorrectly.
> This is the same for getpwuid_r().
Thank you.
I was puzzled by these two sentences in the getgrgid_r man (RHEL6):
"If no matching group record was found, these functions return 0
and store NULL in *result. In case of error, an error number is
returned, and NULL is stored in *result."
Maybe update the man page according with
http://www.gnu.org/software/libc/manual/html_node/Lookup-Group.html. I
think that there the return values are described more clear.
What do you think?
>
> We should make tst_get_unused_uid/tst_get_unused_gid check all the possible
> return value.
>
> Signed-off-by: Zeng Linggang <zenglg.jy@cn.fujitsu.com>
> ---
> lib/tst_uid_gid.c | 31 +++++++++++++++++++++++++++----
> 1 file changed, 27 insertions(+), 4 deletions(-)
>
> diff --git a/lib/tst_uid_gid.c b/lib/tst_uid_gid.c
> index 3ba1ad6..a835c38 100644
> --- a/lib/tst_uid_gid.c
> +++ b/lib/tst_uid_gid.c
> @@ -16,6 +16,7 @@
> * Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA
> */
>
> +#include <errno.h>
> #include <grp.h>
> #include <limits.h>
> #include <pwd.h>
> @@ -44,10 +45,21 @@ uid_t tst_get_unused_uid(void)
> s = getpwuid_r(uid, &pwd, buf, bufsize, &result);
> if (result == NULL) {
> free(buf);
> - if (s == 0)
> + /*
> + * When the given name or gid was not found, getgrgid_r
> + * may return 0 or ENOENT or ESRCH or EBADF or EPERM
> + * or ...
> + */
> + switch (s) {
> + case 0:
> + case ENOENT:
> + case ESRCH:
> + case EBADF:
> + case EPERM:
> return uid;
> - else
> + default:
> return -1;
> + }
> }
> }
>
> @@ -76,10 +88,21 @@ gid_t tst_get_unused_gid(void)
> s = getgrgid_r(gid, &grp, buf, bufsize, &result);
> if (result == NULL) {
> free(buf);
> - if (s == 0)
> + /*
> + * When the given name or gid was not found, getgrgid_r
> + * may return 0 or ENOENT or ESRCH or EBADF or EPERM
> + * or ...
> + */
> + switch (s) {
> + case 0:
> + case ENOENT:
> + case ESRCH:
> + case EBADF:
> + case EPERM:
> return gid;
> - else
> + default:
> return -1;
> + }
> }
> }
>
>
------------------------------------------------------------------------------
Managing the Performance of Cloud-Based Applications
Take advantage of what the Cloud has to offer - Avoid Common Pitfalls.
Read the Whitepaper.
http://pubads.g.doubleclick.net/gampad/clk?id=121054471&iu=/4140/ostg.clktrk
_______________________________________________
Ltp-list mailing list
Ltp-list@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/ltp-list
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [LTP] [PATCH] lib/tst_uid_gid.c: fix checking return value errors about getpwuid_r/getgrgid_r
2014-02-20 7:53 ` Stanislav Kholmanskikh
@ 2014-03-06 11:34 ` Zeng Linggang
2014-03-06 12:36 ` Stanislav Kholmanskikh
0 siblings, 1 reply; 12+ messages in thread
From: Zeng Linggang @ 2014-03-06 11:34 UTC (permalink / raw)
To: Stanislav Kholmanskikh; +Cc: ltp-list
On Thu, 2014-02-20 at 11:53 +0400, Stanislav Kholmanskikh wrote:
> Hi!
>
> On 02/20/2014 06:10 AM, Zeng Linggang wrote:
> > When we call getgrgid_r() to verify whether the corresponding gid is
> > existent, if not, getgrgid_r() will return 0 or ENOENT or ESRCH or
> > EBADF or EPERM...
> >
> > But tst_get_unused_gid() only check return value 0, this is not complete.
> > For example, if the gid is not existent, getgrgid_r() in RHEL7 beta will
> > return ENOENT, which will cause tst_get_unused_gid failed incorrectly.
> > This is the same for getpwuid_r().
>
> Thank you.
>
> I was puzzled by these two sentences in the getgrgid_r man (RHEL6):
> "If no matching group record was found, these functions return 0
> and store NULL in *result. In case of error, an error number is
> returned, and NULL is stored in *result."
>
> Maybe update the man page according with
> http://www.gnu.org/software/libc/manual/html_node/Lookup-Group.html. I
> think that there the return values are described more clear.
>
> What do you think?
>
Hi,
So sorry for the late response.
I think there is a little different in RHEL7.0Beta.
If no gid was found, it returned ENOENT in RHEL7.0Beta.
[root@RHEL7U0Beta_zenglg tmp]# cat test.c
#include <stdlib.h>
#include <sys/types.h>
#include <grp.h>
#include <unistd.h>
#include <limits.h>
int main(void)
{
gid_t gid;
int s;
struct group grp;
char *buf;
long bufsize;
struct group *result;
bufsize = sysconf(_SC_GETGR_R_SIZE_MAX);
if (bufsize == -1)
bufsize = 16384;
buf = malloc(bufsize);
if (buf == NULL)
return -1;
for (gid = 0; gid <= UINT_MAX - 1; gid++) {
s = getgrgid_r(gid, &grp, buf, bufsize, &result);
if (result == NULL) {
free(buf);
printf("s: %d\tgid: %d\n", s, gid);
if (s == 0)
return gid;
else
return -1;
}
}
}
Here is the result that output:
[root@RHEL5U10GA_Intel64 tmp]# ./test
s: 0 gid: 11
[root@RHEL6U5GA_Intel64 tmp]# ./test
s: 0 gid: 13
[root@RHEL7U0Beta_zenglg tmp]# ./test
s: 2 gid: 13
Best regards,
Zeng
------------------------------------------------------------------------------
Subversion Kills Productivity. Get off Subversion & Make the Move to Perforce.
With Perforce, you get hassle-free workflows. Merge that actually works.
Faster operations. Version large binaries. Built-in WAN optimization and the
freedom to use Git, Perforce or both. Make the move to Perforce.
http://pubads.g.doubleclick.net/gampad/clk?id=122218951&iu=/4140/ostg.clktrk
_______________________________________________
Ltp-list mailing list
Ltp-list@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/ltp-list
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [LTP] [PATCH] lib/tst_uid_gid.c: fix checking return value errors about getpwuid_r/getgrgid_r
2014-03-06 11:34 ` Zeng Linggang
@ 2014-03-06 12:36 ` Stanislav Kholmanskikh
2014-03-10 8:13 ` Zeng Linggang
0 siblings, 1 reply; 12+ messages in thread
From: Stanislav Kholmanskikh @ 2014-03-06 12:36 UTC (permalink / raw)
To: Zeng Linggang; +Cc: ltp-list
On 06.03.2014 15:34, Zeng Linggang wrote:
> On Thu, 2014-02-20 at 11:53 +0400, Stanislav Kholmanskikh wrote:
>> Hi!
>>
>> On 02/20/2014 06:10 AM, Zeng Linggang wrote:
>>> When we call getgrgid_r() to verify whether the corresponding gid is
>>> existent, if not, getgrgid_r() will return 0 or ENOENT or ESRCH or
>>> EBADF or EPERM...
>>>
>>> But tst_get_unused_gid() only check return value 0, this is not complete.
>>> For example, if the gid is not existent, getgrgid_r() in RHEL7 beta will
>>> return ENOENT, which will cause tst_get_unused_gid failed incorrectly.
>>> This is the same for getpwuid_r().
>>
>> Thank you.
>>
>> I was puzzled by these two sentences in the getgrgid_r man (RHEL6):
>> "If no matching group record was found, these functions return 0
>> and store NULL in *result. In case of error, an error number is
>> returned, and NULL is stored in *result."
>>
>> Maybe update the man page according with
>> http://www.gnu.org/software/libc/manual/html_node/Lookup-Group.html. I
>> think that there the return values are described more clear.
>>
>> What do you think?
>>
>
> Hi,
> So sorry for the late response.
> I think there is a little different in RHEL7.0Beta.
> If no gid was found, it returned ENOENT in RHEL7.0Beta.
>
Yes, I got this difference at that time:)
I don't have any objections/comments about your patch. I just wanted to
mention one logic "collision" (for me) in the man page. After reading
this page it's not clear whether we should treat the "no matching group
record" situation as a special case or as one of possible errors. But
glibc documentation makes everything clear.
------------------------------------------------------------------------------
Subversion Kills Productivity. Get off Subversion & Make the Move to Perforce.
With Perforce, you get hassle-free workflows. Merge that actually works.
Faster operations. Version large binaries. Built-in WAN optimization and the
freedom to use Git, Perforce or both. Make the move to Perforce.
http://pubads.g.doubleclick.net/gampad/clk?id=122218951&iu=/4140/ostg.clktrk
_______________________________________________
Ltp-list mailing list
Ltp-list@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/ltp-list
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [LTP] [PATCH] lib/tst_uid_gid.c: fix checking return value errors about getpwuid_r/getgrgid_r
2014-03-06 12:36 ` Stanislav Kholmanskikh
@ 2014-03-10 8:13 ` Zeng Linggang
0 siblings, 0 replies; 12+ messages in thread
From: Zeng Linggang @ 2014-03-10 8:13 UTC (permalink / raw)
To: Stanislav Kholmanskikh; +Cc: ltp-list
On Thu, 2014-03-06 at 16:36 +0400, Stanislav Kholmanskikh wrote:
>
> On 06.03.2014 15:34, Zeng Linggang wrote:
> > On Thu, 2014-02-20 at 11:53 +0400, Stanislav Kholmanskikh wrote:
> >> Hi!
> >>
> >> On 02/20/2014 06:10 AM, Zeng Linggang wrote:
> >>> When we call getgrgid_r() to verify whether the corresponding gid is
> >>> existent, if not, getgrgid_r() will return 0 or ENOENT or ESRCH or
> >>> EBADF or EPERM...
> >>>
> >>> But tst_get_unused_gid() only check return value 0, this is not complete.
> >>> For example, if the gid is not existent, getgrgid_r() in RHEL7 beta will
> >>> return ENOENT, which will cause tst_get_unused_gid failed incorrectly.
> >>> This is the same for getpwuid_r().
> >>
> >> Thank you.
> >>
> >> I was puzzled by these two sentences in the getgrgid_r man (RHEL6):
> >> "If no matching group record was found, these functions return 0
> >> and store NULL in *result. In case of error, an error number is
> >> returned, and NULL is stored in *result."
> >>
> >> Maybe update the man page according with
> >> http://www.gnu.org/software/libc/manual/html_node/Lookup-Group.html. I
> >> think that there the return values are described more clear.
> >>
> >> What do you think?
> >>
> >
> > Hi,
> > So sorry for the late response.
> > I think there is a little different in RHEL7.0Beta.
> > If no gid was found, it returned ENOENT in RHEL7.0Beta.
> >
>
> Yes, I got this difference at that time:)
> I don't have any objections/comments about your patch. I just wanted to
> mention one logic "collision" (for me) in the man page. After reading
> this page it's not clear whether we should treat the "no matching group
> record" situation as a special case or as one of possible errors. But
> glibc documentation makes everything clear.
>
:)
OK, thanks. I know what you mean. The man page for getgrgid_r() really
makes us puzzle. I also think the glibc documentation description " If
no group is found or if an error occurred, the pointer returned in
result is a null pointer. The function returns zero or an error code" is
not clear enough. I think the correct implementation should be "If no
group found, this function returns 0 and sets result to a null pointer.
If an error occurs, this function should return an error code". I think
it would be more accurate.
At the same time, I notice the difference in /etc/nsswitch.conf. In
RHEL6.5GA, for group name service, the configure is "group: files",
but in RHEL7.0beta, this configure is "group: files sss". It seems
that service sssd forms this return value difference or the configure
influences the glibc's work.
When I modified the "group: files sss" to "group: files" in
RHEL7.0beta, the test program will return 0.
Thanks.
------------------------------------------------------------------------------
Learn Graph Databases - Download FREE O'Reilly Book
"Graph Databases" is the definitive new guide to graph databases and their
applications. Written by three acclaimed leaders in the field,
this first edition is now available. Download your free book today!
http://p.sf.net/sfu/13534_NeoTech
_______________________________________________
Ltp-list mailing list
Ltp-list@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/ltp-list
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [LTP] [PATCH] lib/tst_uid_gid.c: fix checking return value errors about getpwuid_r/getgrgid_r
2014-02-20 2:10 [LTP] [PATCH] lib/tst_uid_gid.c: fix checking return value errors about getpwuid_r/getgrgid_r Zeng Linggang
2014-02-20 7:53 ` Stanislav Kholmanskikh
@ 2014-03-13 8:33 ` Zeng Linggang
[not found] ` <1396944482.2100.1.camel@G08JYZSD130126>
2 siblings, 0 replies; 12+ messages in thread
From: Zeng Linggang @ 2014-03-13 8:33 UTC (permalink / raw)
To: ltp-list
Hi,
Could any one help to review this patch?
Thanks,
Zeng
On Thu, 2014-02-20 at 10:10 +0800, Zeng Linggang wrote:
> When we call getgrgid_r() to verify whether the corresponding gid is
> existent, if not, getgrgid_r() will return 0 or ENOENT or ESRCH or
> EBADF or EPERM...
>
> But tst_get_unused_gid() only check return value 0, this is not complete.
> For example, if the gid is not existent, getgrgid_r() in RHEL7 beta will
> return ENOENT, which will cause tst_get_unused_gid failed incorrectly.
> This is the same for getpwuid_r().
>
> We should make tst_get_unused_uid/tst_get_unused_gid check all the possible
> return value.
>
> Signed-off-by: Zeng Linggang <zenglg.jy@cn.fujitsu.com>
> ---
> lib/tst_uid_gid.c | 31 +++++++++++++++++++++++++++----
> 1 file changed, 27 insertions(+), 4 deletions(-)
>
> diff --git a/lib/tst_uid_gid.c b/lib/tst_uid_gid.c
> index 3ba1ad6..a835c38 100644
> --- a/lib/tst_uid_gid.c
> +++ b/lib/tst_uid_gid.c
> @@ -16,6 +16,7 @@
> * Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA
> */
>
> +#include <errno.h>
> #include <grp.h>
> #include <limits.h>
> #include <pwd.h>
> @@ -44,10 +45,21 @@ uid_t tst_get_unused_uid(void)
> s = getpwuid_r(uid, &pwd, buf, bufsize, &result);
> if (result == NULL) {
> free(buf);
> - if (s == 0)
> + /*
> + * When the given name or gid was not found, getgrgid_r
> + * may return 0 or ENOENT or ESRCH or EBADF or EPERM
> + * or ...
> + */
> + switch (s) {
> + case 0:
> + case ENOENT:
> + case ESRCH:
> + case EBADF:
> + case EPERM:
> return uid;
> - else
> + default:
> return -1;
> + }
> }
> }
>
> @@ -76,10 +88,21 @@ gid_t tst_get_unused_gid(void)
> s = getgrgid_r(gid, &grp, buf, bufsize, &result);
> if (result == NULL) {
> free(buf);
> - if (s == 0)
> + /*
> + * When the given name or gid was not found, getgrgid_r
> + * may return 0 or ENOENT or ESRCH or EBADF or EPERM
> + * or ...
> + */
> + switch (s) {
> + case 0:
> + case ENOENT:
> + case ESRCH:
> + case EBADF:
> + case EPERM:
> return gid;
> - else
> + default:
> return -1;
> + }
> }
> }
>
------------------------------------------------------------------------------
Learn Graph Databases - Download FREE O'Reilly Book
"Graph Databases" is the definitive new guide to graph databases and their
applications. Written by three acclaimed leaders in the field,
this first edition is now available. Download your free book today!
http://p.sf.net/sfu/13534_NeoTech
_______________________________________________
Ltp-list mailing list
Ltp-list@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/ltp-list
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [LTP] [PATCH] lib/tst_uid_gid.c: fix checking return value errors about getpwuid_r/getgrgid_r
[not found] ` <1396944482.2100.1.camel@G08JYZSD130126>
@ 2014-04-08 11:19 ` chrubis
[not found] ` <5344069B.8010101@oracle.com>
0 siblings, 1 reply; 12+ messages in thread
From: chrubis @ 2014-04-08 11:19 UTC (permalink / raw)
To: Zeng Linggang; +Cc: ltp-list
Hi!
Applied for now, thanks.
Howvever if I remeber correctly the past discussion we do not strictly
need system unused uid and gid in the setregid02 and setregid03
testcases and the failures were caused by the clash between nobody uid
and arbitrary constatnt hardcoded into the source.
If I understand this correctly all need is a uid/gid different from the
nobody user used for the testing, right?
--
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
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [LTP] [PATCH] lib/tst_uid_gid.c: fix checking return value errors about getpwuid_r/getgrgid_r
[not found] ` <5344069B.8010101@oracle.com>
@ 2014-04-08 14:49 ` chrubis
2014-04-08 15:52 ` chrubis
0 siblings, 1 reply; 12+ messages in thread
From: chrubis @ 2014-04-08 14:49 UTC (permalink / raw)
To: Stanislav Kholmanskikh; +Cc: ltp-list
Hi!
> > Applied for now, thanks.
> >
> > Howvever if I remeber correctly the past discussion we do not strictly
> > need system unused uid and gid in the setregid02 and setregid03
> > testcases and the failures were caused by the clash between nobody uid
> > and arbitrary constatnt hardcoded into the source.
>
> setregid03 doesn't use GET_UNUSED_GID(). That time we talked about
> setregid02 and setresuid03.
Right, git grep GET_UNUSED shows these two.
> > If I understand this correctly all need is a uid/gid different from the
> > nobody user used for the testing, right?
> >
> Yes.
>
> And, actually, bin.gr_gid is equivalent to inval_user for purposes of
> setregid02 testing.
>
> Therefore, theoretically, we can cut off:
> &inval_user, &neg_one, EINVAL, &nobody, &nobody,
> "After setregid(invalid group, -1),"}, {
> &neg_one, &inval_user, EINVAL, &nobody, &nobody,
> "After setregid(-1, invalid group),"},};
>
> from there.
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.
--
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
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [LTP] [PATCH] lib/tst_uid_gid.c: fix checking return value errors about getpwuid_r/getgrgid_r
2014-04-08 14:49 ` chrubis
@ 2014-04-08 15:52 ` chrubis
[not found] ` <5344E5C8.3070306@oracle.com>
0 siblings, 1 reply; 12+ messages in thread
From: chrubis @ 2014-04-08 15:52 UTC (permalink / raw)
To: Stanislav Kholmanskikh; +Cc: ltp-list
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
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [LTP] [PATCH] lib/tst_uid_gid.c: fix checking return value errors about getpwuid_r/getgrgid_r
[not found] ` <5344E5C8.3070306@oracle.com>
@ 2014-04-09 13:08 ` chrubis
[not found] ` <53454939.2030704@oracle.com>
0 siblings, 1 reply; 12+ messages in thread
From: chrubis @ 2014-04-09 13:08 UTC (permalink / raw)
To: Stanislav Kholmanskikh; +Cc: ltp-list
Hi!
>
> Thank you very-very much.
>
> It's not bad that we included GET_UNUSED* in LTP. Maybe somebody someday
> will need it for their purposes... :)
>
I'm personaly for removing any unused code because
* you may readd it later when needed because it's in the git history
* there is a price for mantaining each line of code we have
* and any unused code tend to break sooner or later
So I would rather see it removed now and I promise to readd it when we
found a purpose for it.
--
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
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [LTP] [PATCH] lib/tst_uid_gid.c: fix checking return value errors about getpwuid_r/getgrgid_r
[not found] ` <53454939.2030704@oracle.com>
@ 2014-04-09 13:23 ` chrubis
2014-04-09 14:39 ` chrubis
0 siblings, 1 reply; 12+ messages in thread
From: chrubis @ 2014-04-09 13:23 UTC (permalink / raw)
To: Stanislav Kholmanskikh; +Cc: ltp-list
Hi!
> >> Thank you very-very much.
> >>
> >> It's not bad that we included GET_UNUSED* in LTP. Maybe somebody someday
> >> will need it for their purposes... :)
> >>
> >
> > I'm personaly for removing any unused code because
> >
> > * you may readd it later when needed because it's in the git history
> > * there is a price for mantaining each line of code we have
> > * and any unused code tend to break sooner or later
> >
> > So I would rather see it removed now and I promise to readd it when we
> > found a purpose for it.
> >
> No problem.
>
> Just to make it clear. Are you going to remove it by yourself? Or do you
> want me to prepare a patch for this?
>
> Actually, I would like the first option :)
OK, I will remove them.
--
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
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [LTP] [PATCH] lib/tst_uid_gid.c: fix checking return value errors about getpwuid_r/getgrgid_r
2014-04-09 13:23 ` chrubis
@ 2014-04-09 14:39 ` chrubis
0 siblings, 0 replies; 12+ messages in thread
From: chrubis @ 2014-04-09 14:39 UTC (permalink / raw)
To: Stanislav Kholmanskikh; +Cc: ltp-list
Hi!
> > Just to make it clear. Are you going to remove it by yourself? Or do you
> > want me to prepare a patch for this?
> >
> > Actually, I would like the first option :)
>
> OK, I will remove them.
And, done.
--
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
^ permalink raw reply [flat|nested] 12+ messages in thread
end of thread, other threads:[~2014-04-09 14:40 UTC | newest]
Thread overview: 12+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2014-02-20 2:10 [LTP] [PATCH] lib/tst_uid_gid.c: fix checking return value errors about getpwuid_r/getgrgid_r Zeng Linggang
2014-02-20 7:53 ` Stanislav Kholmanskikh
2014-03-06 11:34 ` Zeng Linggang
2014-03-06 12:36 ` Stanislav Kholmanskikh
2014-03-10 8:13 ` Zeng Linggang
2014-03-13 8:33 ` Zeng Linggang
[not found] ` <1396944482.2100.1.camel@G08JYZSD130126>
2014-04-08 11:19 ` chrubis
[not found] ` <5344069B.8010101@oracle.com>
2014-04-08 14:49 ` chrubis
2014-04-08 15:52 ` chrubis
[not found] ` <5344E5C8.3070306@oracle.com>
2014-04-09 13:08 ` chrubis
[not found] ` <53454939.2030704@oracle.com>
2014-04-09 13:23 ` chrubis
2014-04-09 14:39 ` chrubis
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox