[LTP] [PATCH] syscalls/setgroups: fix implicit SETGROUPS parameter casting

[LTP] [PATCH] syscalls/setgroups: fix implicit SETGROUPS parameter casting

[Stanislav Kholmanskikh]

One of parameters to setgroups() syscall is (gid_t *) pointer.
If TST_USE_COMPAT16_VSYSCALL is defined a pointer to GID_T is passed
instead (and sizeof(GID_T) < sizeof(gid_t)). It's not safe and
can result in unaligned access (and SIGBUS) on several platforms.

Signed-off-by: Stanislav Kholmanskikh <stanislav.kholmanskikh@oracle.com>
---
 testcases/kernel/syscalls/setgroups/compat_16.h   |   22 +++++++++++++++++++-
 testcases/kernel/syscalls/setgroups/setgroups04.c |    6 ++++-
 2 files changed, 25 insertions(+), 3 deletions(-)

diff --git a/testcases/kernel/syscalls/setgroups/compat_16.h b/testcases/kernel/syscalls/setgroups/compat_16.h
index 0de4e78..35723d6 100644
--- a/testcases/kernel/syscalls/setgroups/compat_16.h
+++ b/testcases/kernel/syscalls/setgroups/compat_16.h
@@ -32,9 +32,27 @@ extern void cleanup(void);
 #ifdef TST_USE_COMPAT16_SYSCALL
 
 long
-SETGROUPS(size_t gidsetsize, GID_T *list)
+SETGROUPS(size_t gidsetsize, GID_T *list16)
 {
-	return ltp_syscall(__NR_setgroups, gidsetsize, list);
+	int r;
+	int i;
+
+	gid_t *list32;
+
+	list32 = calloc(gidsetsize, sizeof(gid_t));
+	if (list32 == NULL)
+		tst_brkm(TBROK | TERRNO, NULL,
+			"calloc failed to allocate %zu bytes at %s:%d",
+			gidsetsize * sizeof(gid_t),
+			__FILE__, __LINE__);
+
+	for (i = 0; i < gidsetsize; i++)
+		list32[i] = list16[i];
+
+	r = ltp_syscall(__NR_setgroups, gidsetsize, list32);
+
+	free(list32);
+	return r;
 }
 
 int
diff --git a/testcases/kernel/syscalls/setgroups/setgroups04.c b/testcases/kernel/syscalls/setgroups/setgroups04.c
index 5932b4e..42ddda2 100644
--- a/testcases/kernel/syscalls/setgroups/setgroups04.c
+++ b/testcases/kernel/syscalls/setgroups/setgroups04.c
@@ -111,7 +111,11 @@ int main(int ac, char **av)
 		 * verify that it fails with -1 return value and
 		 * sets appropriate errno.
 		 */
-		TEST(SETGROUPS(gidsetsize, sbrk(0)));
+#ifdef TST_USE_COMPAT16_SYSCALL
+		TEST(ltp_syscall(__NR_setgroups, gidsetsize, sbrk(0)));
+#else
+		TEST(setgroups(gidsetsize, sbrk(0)));
+#endif
 
 		if (TEST_RETURN != -1) {
 			tst_resm(TFAIL, "setgroups() returned %ld, "
-- 
1.7.1


------------------------------------------------------------------------------
Get 100% visibility into Java/.NET code with AppDynamics Lite!
It's a free troubleshooting tool designed for production.
Get down to code-level detail for bottlenecks, with <2% overhead. 
Download for free and get started troubleshooting in minutes. 
http://pubads.g.doubleclick.net/gampad/clk?id=48897031&iu=/4140/ostg.clktrk
_______________________________________________
Ltp-list mailing list
Ltp-list@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/ltp-list

Re: [LTP] [PATCH] syscalls/setgroups: fix implicit SETGROUPS parameter casting

[chrubis]

> One of parameters to setgroups() syscall is (gid_t *) pointer.
> If TST_USE_COMPAT16_VSYSCALL is defined a pointer to GID_T is passed
> instead (and sizeof(GID_T) < sizeof(gid_t)). It's not safe and
> can result in unaligned access (and SIGBUS) on several platforms.
> 
> Signed-off-by: Stanislav Kholmanskikh <stanislav.kholmanskikh@oracle.com>
> ---
>  testcases/kernel/syscalls/setgroups/compat_16.h   |   22 +++++++++++++++++++-
>  testcases/kernel/syscalls/setgroups/setgroups04.c |    6 ++++-
>  2 files changed, 25 insertions(+), 3 deletions(-)
> 
> diff --git a/testcases/kernel/syscalls/setgroups/compat_16.h b/testcases/kernel/syscalls/setgroups/compat_16.h
> index 0de4e78..35723d6 100644
> --- a/testcases/kernel/syscalls/setgroups/compat_16.h
> +++ b/testcases/kernel/syscalls/setgroups/compat_16.h
> @@ -32,9 +32,27 @@ extern void cleanup(void);
>  #ifdef TST_USE_COMPAT16_SYSCALL
>  
>  long
> -SETGROUPS(size_t gidsetsize, GID_T *list)
> +SETGROUPS(size_t gidsetsize, GID_T *list16)
>  {
> -	return ltp_syscall(__NR_setgroups, gidsetsize, list);
> +	int r;
> +	int i;
> +
> +	gid_t *list32;
> +
> +	list32 = calloc(gidsetsize, sizeof(gid_t));
> +	if (list32 == NULL)
> +		tst_brkm(TBROK | TERRNO, NULL,
> +			"calloc failed to allocate %zu bytes at %s:%d",
> +			gidsetsize * sizeof(gid_t),
> +			__FILE__, __LINE__);
> +
> +	for (i = 0; i < gidsetsize; i++)
> +		list32[i] = list16[i];
> +
> +	r = ltp_syscall(__NR_setgroups, gidsetsize, list32);
> +
> +	free(list32);
> +	return r;
>  }

This looks like the __NR_setgroups is not the compact16 one we want.

Look at the getgroups16 in kernel/uid16.c it calls groups16_from_user()
and that does:


        for (i = 0; i < group_info->ngroups; i++) {
                if (get_user(group, grouplist+i))
                        return  -EFAULT;


The grouplist is of old_gid_t __user *grouplist type so it's unsigned short, so
it works with array of unsigned short which is aligned to unsigned shorts due
to definiton of the GID_T in the test sources, there is no way this
would trigger unaligned access.

And actually the GETGROUPS() seems to be coded to pass list32 and converts it
to list16 which looks just wrong to me as it's inside of
TST_USE_COMPAT16_SYSCALL. It just looks to me like we have wrong syscall
number to begin with.

-- 
Cyril Hrubis
chrubis@suse.cz

------------------------------------------------------------------------------
Introducing Performance Central, a new site from SourceForge and 
AppDynamics. Performance Central is your source for news, insights, 
analysis and resources for efficient Application Performance Management. 
Visit us today!
http://pubads.g.doubleclick.net/gampad/clk?id=48897511&iu=/4140/ostg.clktrk
_______________________________________________
Ltp-list mailing list
Ltp-list@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/ltp-list

Re: [LTP] [PATCH] syscalls/setgroups: fix implicit SETGROUPS parameter casting

[chrubis]

Hi!
> Thank you for this point.
> 
> At first sight it seems that this testcase is very specific for i386 
> (maybe for other architectures also).
> 
> In i386 I found two variants of setgroups syscall (from 
> arch/x86/syscalls/syscall_32.tbl):
>      81      i386    setgroups               sys_setgroups16
>      206     i386    setgroups32             sys_setgroups
> 
> I guess in i386 glibc handler for setgroups() internally uses 
> _setgroups32_ syscall.
> 
> In x86_64 there is only one variant (from arch/x86/syscalls/syscall_64.tbl):
>      116     common  setgroups               sys_setgroups
> 
> which points to 32-bit version.
> 
> But It's just an assumption and I'll verify this.

The 16 variant is backward compatibility for old programs using this
binari ABI so it makes sense that it does not exist on architectures
that Linux were ported to after the 32 bit syscall was created.

-- 
Cyril Hrubis
chrubis@suse.cz

------------------------------------------------------------------------------
Introducing Performance Central, a new site from SourceForge and 
AppDynamics. Performance Central is your source for news, insights, 
analysis and resources for efficient Application Performance Management. 
Visit us today!
http://pubads.g.doubleclick.net/gampad/clk?id=48897511&iu=/4140/ostg.clktrk
_______________________________________________
Ltp-list mailing list
Ltp-list@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/ltp-list

Re: [LTP] [PATCH] syscalls/setgroups: fix implicit SETGROUPS parameter casting

[chrubis]

Hi!
> I looked at arch/* from linux sources and prepared this table:
> 
> Architecture | __NR_setgroups32 defined | __NR_setgroups defined | glibc 
> for setgroups() invokes |
> alpha            |     | + |
> arm              | + | + |
> avr32           |     | + |
> blackfin        | + | + |
> cris               | + | + |
> frv                | + | + |
> h8300           | + | + |
> ia64              | + |   |
> m32r            | + |   |
> m68k            | + | + |
> microblaze   | + | + |
> mips             |    | + |
> mn10300     | + | + |
> parisc           |    | + |
> powerpc       |    | + | setgroups   |
> s390             | + | + | setgroups32 |
> s390x           |     | + | setgroups   |
> sh-32            | + | + | setgroups32 |
> sh-64            | + | + | setgroups32 |
> sparc32        | + | + | setgroups32 |
> sparc64        |     | + | setgroups   |
> i386              | + | + | setgroups32 |
> x86_64         |     | + | setgroups   |
> xtensa          |    | + |
> 
> Sorry for formatting.
> 
> It looks like that if __NR_setgroups32 is defined then this means that 
> syscall pointed by __NR_setgroups is 16bit.
> 
> So how do you think about this patch ?
> 
> diff --git a/testcases/kernel/syscalls/setgroups/compat_16.h 
> b/testcases/kernel/syscalls/setgroups/compat_16.h
> index 0de4e78..8f46da9 100644
> --- a/testcases/kernel/syscalls/setgroups/compat_16.h
> +++ b/testcases/kernel/syscalls/setgroups/compat_16.h
> @@ -32,9 +32,16 @@ extern void cleanup(void);
>   #ifdef TST_USE_COMPAT16_SYSCALL
> 
>   long
> -SETGROUPS(size_t gidsetsize, GID_T *list)
> +SETGROUPS(size_t gidsetsize, GID_T *list16)
>   {
> -       return ltp_syscall(__NR_setgroups, gidsetsize, list);
> +# if (__NR_setgroups32 != __LTP__NR_INVALID_SYSCALL)
> +       /* __NR_setgroups - 16-bit version of setgroups() syscall */
> +       return ltp_syscall(__NR_setgroups, gidsetsize, list16);
> +# else
> +       /* The platform has no 16-bit version of setgroups() syscall */
> +       tst_brkm(TCONF, NULL,
> +               "16-bit version of setgroups() is not supported on your 
> arch");
> +# endif /* __NR_setgroups32 */

Looks good to me.

And we should fix the getgroups in the same header as well.

-- 
Cyril Hrubis
chrubis@suse.cz

------------------------------------------------------------------------------
Introducing Performance Central, a new site from SourceForge and 
AppDynamics. Performance Central is your source for news, insights, 
analysis and resources for efficient Application Performance Management. 
Visit us today!
http://pubads.g.doubleclick.net/gampad/clk?id=48897511&iu=/4140/ostg.clktrk
_______________________________________________
Ltp-list mailing list
Ltp-list@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/ltp-list

Re: [LTP] [PATCH] syscalls/setgroups: fix implicit SETGROUPS parameter casting

[chrubis]

Hi!
> But we can't rely on the fact that group ids of a user invoking the 
> testcases are fit into __old_gid_t.
> 
> And the purpose of current GETGROUPS definition is to handle such cases.
> 
> A situation may happen where root user is a member of a group with a 
> _big_ group id.
> 
> So I'm not 100% sure about fixing getgroups part of the same header.
> 
> Or did I miss something?

What I mean is that we should check if the compatibility getgroups
syscall exists and if not abort the test (as the setgroups will do).
Because the 32 bit getgroups syscall is tested with the tests without
the TST_USE_COMPAT16_SYSCALL.

-- 
Cyril Hrubis
chrubis@suse.cz

------------------------------------------------------------------------------
Introducing Performance Central, a new site from SourceForge and 
AppDynamics. Performance Central is your source for news, insights, 
analysis and resources for efficient Application Performance Management. 
Visit us today!
http://pubads.g.doubleclick.net/gampad/clk?id=48897511&iu=/4140/ostg.clktrk
_______________________________________________
Ltp-list mailing list
Ltp-list@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/ltp-list

[LTP] [PATCH V2 1/2] syscalls/setgroups: fix 16-bit versions of the testcases

[Stanislav Kholmanskikh]

Now 16-bit testcases are executed only if platform
has corresponding 16-bit version of setgroups() syscall.

Signed-off-by: Stanislav Kholmanskikh <stanislav.kholmanskikh@oracle.com>
---
 testcases/kernel/syscalls/setgroups/compat_16.h |   11 +++++++++--
 1 files changed, 9 insertions(+), 2 deletions(-)

diff --git a/testcases/kernel/syscalls/setgroups/compat_16.h b/testcases/kernel/syscalls/setgroups/compat_16.h
index 0de4e78..8f46da9 100644
--- a/testcases/kernel/syscalls/setgroups/compat_16.h
+++ b/testcases/kernel/syscalls/setgroups/compat_16.h
@@ -32,9 +32,16 @@ extern void cleanup(void);
 #ifdef TST_USE_COMPAT16_SYSCALL
 
 long
-SETGROUPS(size_t gidsetsize, GID_T *list)
+SETGROUPS(size_t gidsetsize, GID_T *list16)
 {
-	return ltp_syscall(__NR_setgroups, gidsetsize, list);
+# if (__NR_setgroups32 != __LTP__NR_INVALID_SYSCALL)
+	/* __NR_setgroups - 16-bit version of setgroups() syscall */
+	return ltp_syscall(__NR_setgroups, gidsetsize, list16);
+# else
+	/* The platform has no 16-bit version of setgroups() syscall */
+	tst_brkm(TCONF, NULL,
+		"16-bit version of setgroups() is not supported on your arch");
+# endif /* __NR_setgroups32 */
 }
 
 int
-- 
1.7.1


------------------------------------------------------------------------------
Introducing Performance Central, a new site from SourceForge and 
AppDynamics. Performance Central is your source for news, insights, 
analysis and resources for efficient Application Performance Management. 
Visit us today!
http://pubads.g.doubleclick.net/gampad/clk?id=48897511&iu=/4140/ostg.clktrk
_______________________________________________
Ltp-list mailing list
Ltp-list@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/ltp-list

[LTP] [PATCH V2 2/2] syscalls/getgroups: added checks for 16-bit getgroups() syscall

[Stanislav Kholmanskikh]

getgroups01_16 and getgroups03_16 did not use 16-bit
version of getgroups() syscall. Fixed this in the same manner as
syscalls/setgroups.

Signed-off-by: Stanislav Kholmanskikh <stanislav.kholmanskikh@oracle.com>
---
 testcases/kernel/syscalls/getgroups/compat_16.h   |   78 +++++++++++++++++++++
 testcases/kernel/syscalls/getgroups/getgroups01.c |   22 +++---
 testcases/kernel/syscalls/getgroups/getgroups03.c |   17 +++--
 3 files changed, 99 insertions(+), 18 deletions(-)
 create mode 100644 testcases/kernel/syscalls/getgroups/compat_16.h

diff --git a/testcases/kernel/syscalls/getgroups/compat_16.h b/testcases/kernel/syscalls/getgroups/compat_16.h
new file mode 100644
index 0000000..79d006e
--- /dev/null
+++ b/testcases/kernel/syscalls/getgroups/compat_16.h
@@ -0,0 +1,78 @@
+/*
+ * Copyright (c) 2013 Oracle and/or its affiliates. All Rights Reserved.
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU General Public License as
+ * published by the Free Software Foundation; either version 2 of
+ * the License, or (at your option) any later version.
+ *
+ * This program is distributed in the hope that it would be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program; if not, write the Free Software Foundation,
+ * Inc.,  51 Franklin St, Fifth Floor, Boston, MA  02110-1301  USA
+ *
+ * Author: Stanislav Kholmanskikh <stanislav.kholmanskikh@oracle.com>
+ *
+ */
+
+#ifndef __GETGROUPS_COMPAT_16_H__
+#define __GETGROUPS_COMPAT_16_H__
+
+#include "compat_gid.h"
+#include "linux_syscall_numbers.h"
+
+#ifdef TST_USE_COMPAT16_SYSCALL
+
+int SETGROUPS(size_t size, const GID_T *list16)
+{
+	int r;
+	int i;
+
+	gid_t *list32;
+	list32 = calloc(size, sizeof(gid_t));
+	if (list32 == NULL)
+		tst_brkm(TBROK | TERRNO, NULL,
+			"calloc() failed to allocate %zu bytes",
+			size * sizeof(gid_t));
+
+	for (i = 0; i < size; i++)
+		list32[i] = list16[i];
+
+	r = setgroups(size, list32);
+
+	free(list32);
+
+	return r;
+}
+
+int GETGROUPS(int size, GID_T *list16)
+{
+# if (__NR_getgroups32 != __LTP__NR_INVALID_SYSCALL)
+	/* __NR_getgroups - 16-bit version of getgroups() syscall */
+	return ltp_syscall(__NR_getgroups, size, list16);
+# else
+	/* The platform has no 16-bit version of getgroups() syscall */
+	tst_brkm(TCONF, NULL,
+		"16-bit version of getgroups() is not supported on your arch");
+# endif /* __NR_getgroups32 */
+}
+
+#else
+
+int SETGROUPS(size_t size, const GID_T *list32)
+{
+	return setgroups(size, list32);
+}
+
+int GETGROUPS(int size, GID_T *list32)
+{
+	return getgroups(size, list32);
+}
+
+#endif /* TST_USE_COMPAT16_SYSCALL */
+
+#endif /* __GETGROUPS_COMPAT_16_H__ */
diff --git a/testcases/kernel/syscalls/getgroups/getgroups01.c b/testcases/kernel/syscalls/getgroups/getgroups01.c
index 1d58109..39157aa 100644
--- a/testcases/kernel/syscalls/getgroups/getgroups01.c
+++ b/testcases/kernel/syscalls/getgroups/getgroups01.c
@@ -52,8 +52,10 @@
 #include <grp.h>
 #include <sys/param.h>
 #include <sys/types.h>
+
 #include "test.h"
 #include "usctest.h"
+#include "compat_16.h"
 
 static void setup(void);
 static void cleanup(void);
@@ -61,14 +63,14 @@ static void cleanup(void);
 char *TCID = "getgroups01";
 int TST_TOTAL = 4;
 
-static gid_t gidset[NGROUPS];
-static gid_t cmpset[NGROUPS];
+static GID_T gidset[NGROUPS];
+static GID_T cmpset[NGROUPS];
 
 int main(int ac, char **av)
 {
 	int lc;
 	char *msg;
-	gid_t group;
+	GID_T group;
 	int i;
 	int entries;
 
@@ -83,7 +85,7 @@ int main(int ac, char **av)
 
 		tst_count = 0;
 
-		TEST(getgroups(-1, gidset));
+		TEST(GETGROUPS(-1, gidset));
 
 		if (TEST_RETURN == 0) {
 			tst_resm(TFAIL, "getgroups succeeded unexpectedly");
@@ -101,14 +103,14 @@ int main(int ac, char **av)
 		 * return and the the gidset array is not modified.
 		 * This is a POSIX special case.
 		 */
-		memset(gidset, 052, NGROUPS * sizeof(gid_t));
-		memset(cmpset, 052, NGROUPS * sizeof(gid_t));
+		memset(gidset, 052, NGROUPS * sizeof(GID_T));
+		memset(cmpset, 052, NGROUPS * sizeof(GID_T));
 
-		TEST(getgroups(0, gidset));
+		TEST(GETGROUPS(0, gidset));
 		if (TEST_RETURN == -1) {
 			tst_resm(TFAIL | TTERRNO, "getgroups failed unexpectedly");
 		} else if (STD_FUNCTIONAL_TEST) {
-			if (memcmp(cmpset, gidset, NGROUPS * sizeof(gid_t)) != 0)
+			if (memcmp(cmpset, gidset, NGROUPS * sizeof(GID_T)) != 0)
 				tst_resm(TFAIL,
 					 "getgroups modified the gidset array");
 			else
@@ -126,7 +128,7 @@ int main(int ac, char **av)
 				 "getgroups returned %ld; unable to test that using ngrps >=1 but less than number of grps",
 				 TEST_RETURN);
 		} else {
-			TEST(getgroups(TEST_RETURN - 1, gidset));
+			TEST(GETGROUPS(TEST_RETURN - 1, gidset));
 			if (TEST_RETURN == -1) {
 				if (STD_FUNCTIONAL_TEST) {
 					if (errno == EINVAL)
@@ -145,7 +147,7 @@ int main(int ac, char **av)
 			}
 		}
 
-		TEST(getgroups(NGROUPS, gidset));
+		TEST(GETGROUPS(NGROUPS, gidset));
 		if ((entries = TEST_RETURN) == -1) {
 			tst_resm(TFAIL | TTERRNO,
 				 "getgroups failed unexpectedly");
diff --git a/testcases/kernel/syscalls/getgroups/getgroups03.c b/testcases/kernel/syscalls/getgroups/getgroups03.c
index 1e25756..3a0ac0c 100644
--- a/testcases/kernel/syscalls/getgroups/getgroups03.c
+++ b/testcases/kernel/syscalls/getgroups/getgroups03.c
@@ -41,6 +41,7 @@
 
 #include "test.h"
 #include "usctest.h"
+#include "compat_16.h"
 
 #define TESTUSER "root"
 
@@ -48,8 +49,8 @@ char *TCID = "getgroups03";
 int TST_TOTAL = 1;
 
 static int ngroups;
-static gid_t groups_list[NGROUPS];
-static gid_t groups[NGROUPS];
+static GID_T groups_list[NGROUPS];
+static GID_T groups[NGROUPS];
 
 static void verify_groups(int ret_ngroups);
 static void setup(void);
@@ -70,7 +71,7 @@ int main(int ac, char **av)
 
 		tst_count = 0;
 
-		TEST(getgroups(gidsetsize, groups_list));
+		TEST(GETGROUPS(gidsetsize, groups_list));
 
 		if (TEST_RETURN == -1) {
 			tst_resm(TFAIL | TTERRNO, "getgroups failed");
@@ -88,18 +89,18 @@ int main(int ac, char **av)
 }
 
 /*
- * readgroups(gid_t *)  - Read supplimentary group ids of "root" user
+ * readgroups(GID_T *)  - Read supplimentary group ids of "root" user
  * Scans the /etc/group file to get IDs of all the groups to which TESTUSER
  * belongs and puts them into the array passed.
  * Returns the no of gids read.
  */
-static int readgroups(gid_t groups[NGROUPS])
+static int readgroups(GID_T groups[NGROUPS])
 {
 	struct group *grp;
 	int ngrps = 0;
 	int i;
 	int found;
-	gid_t g;
+	GID_T g;
 
 	setgrent();
 
@@ -151,7 +152,7 @@ static void setup(void)
 	 * testcase will fail. So execute setgroups() before executing
 	 * getgroups()
 	 */
-	if (setgroups(ngroups, groups) == -1)
+	if (SETGROUPS(ngroups, groups) == -1)
 		tst_brkm(TBROK | TERRNO, cleanup, "setgroups failed");
 }
 
@@ -165,7 +166,7 @@ static void setup(void)
 static void verify_groups(int ret_ngroups)
 {
 	int i, j;
-	gid_t egid;
+	GID_T egid;
 	int egid_flag = 1;
 	int fflag = 1;
 
-- 
1.7.1


------------------------------------------------------------------------------
Introducing Performance Central, a new site from SourceForge and 
AppDynamics. Performance Central is your source for news, insights, 
analysis and resources for efficient Application Performance Management. 
Visit us today!
http://pubads.g.doubleclick.net/gampad/clk?id=48897511&iu=/4140/ostg.clktrk
_______________________________________________
Ltp-list mailing list
Ltp-list@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/ltp-list

Re: [LTP] [PATCH] syscalls/setgroups: fix implicit SETGROUPS parameter casting

[chrubis]

Hi!
> >> But we can't rely on the fact that group ids of a user invoking the
> >> testcases are fit into __old_gid_t.
> >>
> >> And the purpose of current GETGROUPS definition is to handle such cases.
> >>
> >> A situation may happen where root user is a member of a group with a
> >> _big_ group id.
> >>
> >> So I'm not 100% sure about fixing getgroups part of the same header.
> >>
> >> Or did I miss something?
> > What I mean is that we should check if the compatibility getgroups
> > syscall exists and if not abort the test (as the setgroups will do).
> I understand, but should it be a part of syscalls/setgroups testcases?

Ideally we should move all the syscall magic into more general place,
some subdirectory of include/ I guess.

> Maybe we should modify a little bit syscalls/getgroups tests and add 
> this check there?

That would not help much, to test setgroups() you need to be able to
call getgroups() as well, thus I think that we should move them into
more general location.

And moreover it looks like getgroups testcases currently are not able to
call the compat syscalls at all.

-- 
Cyril Hrubis
chrubis@suse.cz

------------------------------------------------------------------------------
Introducing Performance Central, a new site from SourceForge and 
AppDynamics. Performance Central is your source for news, insights, 
analysis and resources for efficient Application Performance Management. 
Visit us today!
http://pubads.g.doubleclick.net/gampad/clk?id=48897511&iu=/4140/ostg.clktrk
_______________________________________________
Ltp-list mailing list
Ltp-list@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/ltp-list

Re: [LTP] [PATCH V2 2/2] syscalls/getgroups: added checks for 16-bit getgroups() syscall

[chrubis]

Hi!
> Signed-off-by: Stanislav Kholmanskikh <stanislav.kholmanskikh@oracle.com>
> ---
>  testcases/kernel/syscalls/getgroups/compat_16.h   |   78 +++++++++++++++++++++
>  testcases/kernel/syscalls/getgroups/getgroups01.c |   22 +++---
>  testcases/kernel/syscalls/getgroups/getgroups03.c |   17 +++--
>  3 files changed, 99 insertions(+), 18 deletions(-)
>  create mode 100644 testcases/kernel/syscalls/getgroups/compat_16.h
> 
> diff --git a/testcases/kernel/syscalls/getgroups/compat_16.h b/testcases/kernel/syscalls/getgroups/compat_16.h
> new file mode 100644
> index 0000000..79d006e
> --- /dev/null
> +++ b/testcases/kernel/syscalls/getgroups/compat_16.h
> @@ -0,0 +1,78 @@
> +/*
> + * Copyright (c) 2013 Oracle and/or its affiliates. All Rights Reserved.
> + *
> + * This program is free software; you can redistribute it and/or
> + * modify it under the terms of the GNU General Public License as
> + * published by the Free Software Foundation; either version 2 of
> + * the License, or (at your option) any later version.
> + *
> + * This program is distributed in the hope that it would be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> + * GNU General Public License for more details.
> + *
> + * You should have received a copy of the GNU General Public License
> + * along with this program; if not, write the Free Software Foundation,
> + * Inc.,  51 Franklin St, Fifth Floor, Boston, MA  02110-1301  USA
> + *
> + * Author: Stanislav Kholmanskikh <stanislav.kholmanskikh@oracle.com>
> + *
> + */
> +
> +#ifndef __GETGROUPS_COMPAT_16_H__
> +#define __GETGROUPS_COMPAT_16_H__
> +
> +#include "compat_gid.h"
> +#include "linux_syscall_numbers.h"
> +
> +#ifdef TST_USE_COMPAT16_SYSCALL
> +
> +int SETGROUPS(size_t size, const GID_T *list16)
> +{
> +	int r;
> +	int i;
> +
> +	gid_t *list32;
> +	list32 = calloc(size, sizeof(gid_t));
> +	if (list32 == NULL)
> +		tst_brkm(TBROK | TERRNO, NULL,
> +			"calloc() failed to allocate %zu bytes",
> +			size * sizeof(gid_t));
> +
> +	for (i = 0; i < size; i++)
> +		list32[i] = list16[i];
> +
> +	r = setgroups(size, list32);
> +
> +	free(list32);
> +
> +	return r;
> +}

I think that we should just use the 16 bit variant here as well, as
well. It's available if the compat getgroups is and there is no need to
call the 32 bit version.

Or do you want to explicitly assert that the 32 bit syscall returns the
same as the 16 bit? That would make some sense.

> +int GETGROUPS(int size, GID_T *list16)
> +{
> +# if (__NR_getgroups32 != __LTP__NR_INVALID_SYSCALL)
> +	/* __NR_getgroups - 16-bit version of getgroups() syscall */
> +	return ltp_syscall(__NR_getgroups, size, list16);
> +# else
> +	/* The platform has no 16-bit version of getgroups() syscall */
> +	tst_brkm(TCONF, NULL,
> +		"16-bit version of getgroups() is not supported on your arch");
> +# endif /* __NR_getgroups32 */
> +}
> +
> +#else
> +
> +int SETGROUPS(size_t size, const GID_T *list32)
> +{
> +	return setgroups(size, list32);
> +}
> +
> +int GETGROUPS(int size, GID_T *list32)
> +{
> +	return getgroups(size, list32);
> +}
> +
> +#endif /* TST_USE_COMPAT16_SYSCALL */
> +
> +#endif /* __GETGROUPS_COMPAT_16_H__ */

But still it duplicates code in the tree, lets move it into some header
under 'LTP/include/sys/' and move compat_gid.h there as well.

-- 
Cyril Hrubis
chrubis@suse.cz

------------------------------------------------------------------------------
Introducing Performance Central, a new site from SourceForge and 
AppDynamics. Performance Central is your source for news, insights, 
analysis and resources for efficient Application Performance Management. 
Visit us today!
http://pubads.g.doubleclick.net/gampad/clk?id=48897511&iu=/4140/ostg.clktrk
_______________________________________________
Ltp-list mailing list
Ltp-list@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/ltp-list

Re: [LTP] [PATCH V2 2/2] syscalls/getgroups: added checks for 16-bit getgroups() syscall

[chrubis]

Hi!
> Ok. I'm managing this (compat_16.h from several testcases, compat_uid.h, 
> compat_gid.h).
> 
> But I need one more clarification.
> 
> Look at LTP/testcases/kernel/include. There are linux_syscalls_numbers.h 
> and numa_helper.h
> 
> Maybe move all these compat headers not into LTP/include/sys, but into 
> LTP/testcases/kernel/include?
> 
> I think It's a more appropriate directory.
> 
> How do you think?

I'm fine with testcases/kernel/include as well.

-- 
Cyril Hrubis
chrubis@suse.cz

------------------------------------------------------------------------------
Learn the latest--Visual Studio 2012, SharePoint 2013, SQL 2012, more!
Discover the easy way to master current and previous Microsoft technologies
and advance your career. Get an incredible 1,500+ hours of step-by-step
tutorial videos with LearnDevNow. Subscribe today and save!
http://pubads.g.doubleclick.net/gampad/clk?id=58040911&iu=/4140/ostg.clktrk
_______________________________________________
Ltp-list mailing list
Ltp-list@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/ltp-list

Re: [LTP] [PATCH V3 2/3] fixes for 16-bit syscalls testcases

[chrubis]

> +/*
> + * LTP_COMPAT16_IS_DEFINED(SNAME) - if true, then __NR_SNAME
> + *   is a 16-bit version of SNAME() syscall
> + *
> + * LTP_COMPAT16_IF_DEFINED(SNAME, ...) - action to execute
> + *   if 16-bit version of SNAME syscall is supported
> + *
> + * LTP_COMPAT16_IF_UNDEFINED(SNAME) - action to execute
> + *   if 16-bit version of SNAME syscall is not supported
> + */
> +#define LTP_COMPAT16_IS_DEFINED(SNAME) \
> +	(__NR_##SNAME##32 != __LTP__NR_INVALID_SYSCALL)
> +
> +#define LTP_COMPAT16_IF_DEFINED(SNAME, ...) \
> +	return ltp_syscall(__NR_##SNAME, ##__VA_ARGS__);
> +
> +#define LTP_COMPAT16_IF_UNDEFINED(SNAME) \
> +	tst_brkm(TCONF, cleanup, \
> +		"16-bit version of %s() is not supported on your platform", \
> +		#SNAME);

What I do not like is the calling cleanup from this part of the code, it
was not passed there and the code will break badly if you name the
cleanup function differently (or create more of them, cleanup, cleanup1,
...).

What I would do here is to set errno to ENOSYS and return a failure. And
let the test that called the function handle it.

Or we can add a cleanup parameter to the all caps functions bellow.

> +int
> +SETGROUPS(size_t gidsetsize, GID_T *list16)
> +{
> +# if LTP_COMPAT16_IS_DEFINED(setgroups)
> +	LTP_COMPAT16_IF_DEFINED(setgroups, gidsetsize, list16)
> +# else
> +	LTP_COMPAT16_IF_UNDEFINED(setgroups)
> +# endif
> +}
> +
> +int
> +GETGROUPS(int gidsetsize, GID_T *list16)
> +{
> +# if LTP_COMPAT16_IS_DEFINED(getgroups)
> +	LTP_COMPAT16_IF_DEFINED(getgroups, gidsetsize, list16)
> +# else
> +	LTP_COMPAT16_IF_UNDEFINED(getgroups)
> +# endif
> +}
> +
> +#else
> +int
> +SETGROUPS(size_t size, const GID_T *list)
> +{
> +	return setgroups(size, list);
> +}
> +
> +int
> +GETGROUPS(int size, GID_T *list)
> +{
> +	return getgroups(size, list);
> +}
> +
> +#endif	/* TST_USE_COMPAT16_SYSCALL */
> +
> +#endif /* __SETGROUPS_COMPAT_16_H__ */

We can even simplify this macros. At least the innter part of SETGROUPS
and GETGROUPS could be created by a single macro:

COMPAT16_SYSCALL(setgroups, gitsetsize, list16)

#define COMPAT16_SYSCALL(sys_name, ...) \
# ifdef TST_USE_COMPAT16_SYSCALL
#  if ....
	return ltp_syscall(sys_name, ##__VA_ARGS__);
#  else
	errno = ENOSYS;
	return -1;
#  fi
# else
	return sys_name(__VA_ARGS__);
# endif

In case that the syscall and libcall params does not match we can use
only the part enclosed in the ifdef TST_USE_COMPAT16_SYSCALL.

-- 
Cyril Hrubis
chrubis@suse.cz

------------------------------------------------------------------------------
Learn the latest--Visual Studio 2012, SharePoint 2013, SQL 2012, more!
Discover the easy way to master current and previous Microsoft technologies
and advance your career. Get an incredible 1,500+ hours of step-by-step
tutorial videos with LearnDevNow. Subscribe today and save!
http://pubads.g.doubleclick.net/gampad/clk?id=58040911&iu=/4140/ostg.clktrk
_______________________________________________
Ltp-list mailing list
Ltp-list@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/ltp-list

Re: [LTP] [PATCH V3 3/3] syscalls/getgroups: added checks for 16-bit getgroups() syscall

[chrubis]

Hi!
> getgroups01_16 and getgroups03_16 did not use 16-bit
> versions of getgroups() syscall. Fixed this in the same manner like
> syscalls/setgroups.

This part looks fine. 

-- 
Cyril Hrubis
chrubis@suse.cz

------------------------------------------------------------------------------
Learn the latest--Visual Studio 2012, SharePoint 2013, SQL 2012, more!
Discover the easy way to master current and previous Microsoft technologies
and advance your career. Get an incredible 1,500+ hours of step-by-step
tutorial videos with LearnDevNow. Subscribe today and save!
http://pubads.g.doubleclick.net/gampad/clk?id=58040911&iu=/4140/ostg.clktrk
_______________________________________________
Ltp-list mailing list
Ltp-list@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/ltp-list

Re: [LTP] [PATCH V3 1/3] syscalls/utils/compat_16.mk: fix build dependencies

[chrubis]

Hi!
> If we create/modify/touch file syscalls/utils/compat_16.h and execute
> 'make' in any of syscalls directories which includes compat_16.mk, then
> nothing will happen, because this approach:
> 
>    %.c: $(COMPAT_16_H)
> 
> is not working.
> 
> Fixed this.
> 
> Signed-off-by: Stanislav Kholmanskikh <stanislav.kholmanskikh@oracle.com>
> ---
>  testcases/kernel/syscalls/utils/compat_16.mk |    6 ++++--
>  1 files changed, 4 insertions(+), 2 deletions(-)
> 
> diff --git a/testcases/kernel/syscalls/utils/compat_16.mk b/testcases/kernel/syscalls/utils/compat_16.mk
> index 371bd43..36ece1b 100644
> --- a/testcases/kernel/syscalls/utils/compat_16.mk
> +++ b/testcases/kernel/syscalls/utils/compat_16.mk
> @@ -55,6 +55,7 @@ CPPFLAGS		+= -I$(abs_srcdir) -I$(abs_srcdir)/../utils
>  SRCS			?= $(wildcard $(abs_srcdir)/*.c)
>  
>  MAKE_TARGETS		:= $(notdir $(patsubst %.c,%,$(SRCS)))
> +MAKE_TARGETS_OBJS_WO_COMPAT_16	:= $(addsuffix .o,$(MAKE_TARGETS))
>  
>  ifneq ($(TST_COMPAT_16_SYSCALL),no)
>  MAKE_TARGETS		+= $(addsuffix _16,$(MAKE_TARGETS))
> @@ -69,7 +70,8 @@ COMPAT_16_H		:= $(abs_srcdir)/../utils/compat_16.h
>  ifneq ($(wildcard $(COMPAT_16_H)),)
>  HAS_COMPAT_16		:= 1
>  
> -%.c: $(COMPAT_16_H)
> +$(MAKE_TARGETS_OBJS_WO_COMPAT_16): $(COMPAT_16_H)
> +.INTERMEDIATE: $(MAKE_TARGETS_OBJS_WO_COMPAT_16)
>  
>  else
>  HAS_COMPAT_16		:= 0
> @@ -78,5 +80,5 @@ endif
>  %_16: CPPFLAGS += -D$(DEF_16)=1
>  # XXX (garrcoop): End section of code in question..
>  
> -%_16.o: %.c
> +%_16.o: %.c $(COMPAT_16_H)
>  	$(COMPILE.c) $(OUTPUT_OPTION) $<

Perhaps we can do this more cleanly by making the _16 binaries depend on
the compat_16.h as with:

--- a/testcases/kernel/syscalls/utils/compat_16.mk
+++ b/testcases/kernel/syscalls/utils/compat_16.mk
@@ -57,7 +57,8 @@ SRCS			?= $(wildcard $(abs_srcdir)/*.c)
 MAKE_TARGETS		:= $(notdir $(patsubst %.c,%,$(SRCS)))
 
 ifneq ($(TST_COMPAT_16_SYSCALL),no)
-MAKE_TARGETS		+= $(addsuffix _16,$(MAKE_TARGETS))
+MAKE_TARGETS_16	 = $(addsuffix _16,$(MAKE_TARGETS))
+MAKE_TARGETS		+= $(MAKE_TARGETS_16)
 endif
 
 # XXX (garrcoop): This code should be put in question as it cannot be applied
@@ -69,8 +70,7 @@ COMPAT_16_H		:= $(abs_srcdir)/../utils/compat_16.h
 ifneq ($(wildcard $(COMPAT_16_H)),)
 HAS_COMPAT_16		:= 1
 
-%.c: $(COMPAT_16_H)
-
+$(MAKE_TARGETS_16): $(COMPAT_16_H)
 else
 HAS_COMPAT_16		:= 0
 endif

-- 
Cyril Hrubis
chrubis@suse.cz

------------------------------------------------------------------------------
Learn the latest--Visual Studio 2012, SharePoint 2013, SQL 2012, more!
Discover the easy way to master current and previous Microsoft technologies
and advance your career. Get an incredible 1,500+ hours of step-by-step
tutorial videos with LearnDevNow. Subscribe today and save!
http://pubads.g.doubleclick.net/gampad/clk?id=58040911&iu=/4140/ostg.clktrk
_______________________________________________
Ltp-list mailing list
Ltp-list@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/ltp-list

Re: [LTP] [PATCH V3 2/3] fixes for 16-bit syscalls testcases

[chrubis]

Hi!
> Sorry, but is it possible to use #ifdef or #if inside a #define macro?

Right it does not, sorry. I tend to forget how dumb it the C preprocessor is.

> This doesn't work:
> 
> #define MYDEF(NN) \
> # ifdef TST_USE_COMPAT16_SYSCALL
>          return NN(0);
> # else
>          return NN(1);
> # endif
> 
> MYDEF(bla)

We need to pull the first if out of the macro and we can do the second
one as a regular if (it will be optimized away anyway).

#ifdef TST_USE_COMPAT16_SYSCALL
# define LTP_CREATE_SYSCALL(sys_name, ...) \
	if (__NR_##sys_name##32 != __LTP__NR_INVALID_SYSCALL) { \
		return ltp_syscall(sys_name, ##__VA_ARGS__); \
	} else { \
		errno = ENOSYS; \
		return -1; \
	}
#else
# define LTP_CREATE_SYSCALL(sys_name, ...) \
	return sys_name(__VA_ARGS__)
#endif


The usage will be:

int SETGROUPS(size_t size, const GID_T *list)
{
	LTP_CREATE_SYSCALL(getgroups, size, list);
}

Does this sound reasonable?

-- 
Cyril Hrubis
chrubis@suse.cz

------------------------------------------------------------------------------
Learn the latest--Visual Studio 2012, SharePoint 2013, SQL 2012, more!
Discover the easy way to master current and previous Microsoft technologies
and advance your career. Get an incredible 1,500+ hours of step-by-step
tutorial videos with LearnDevNow. Subscribe today and save!
http://pubads.g.doubleclick.net/gampad/clk?id=58040911&iu=/4140/ostg.clktrk
_______________________________________________
Ltp-list mailing list
Ltp-list@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/ltp-list

Re: [LTP] [PATCH V3 1/3] syscalls/utils/compat_16.mk: fix build dependencies

[chrubis]

Hi!
> But I think that my approach is easier to understand: we explicitly 
> define object files targets, make them depend on compat_16.h and also 
> mark them as intermediate targets (so make will remove them after linking).
> 
> And if we make binaries to depend on compat_16.h gcc treats this header 
> file not as a header:
> 
> gcc -g -O2 -g -O2 -fno-strict-aliasing -pipe -Wall 
> -I/home/stas/ltp/testcases/kernel/include 
> -I/home/stas/ltp/testcases/kernel/syscalls/setgroups 
> -I/home/stas/ltp/testcases/kernel/syscalls/setgroups/../utils 
> -I../../../../include -I../../../../include   -L../../../../lib 
> setgroups01.c 
> /home/stas/ltp/testcases/kernel/syscalls/setgroups/../utils/compat_16.h 
> -lltp -o setgroups01
> 
> gcc -g -O2 -g -O2 -fno-strict-aliasing -pipe -Wall 
> -I/home/stas/ltp/testcases/kernel/include 
> -I/home/stas/ltp/testcases/kernel/syscalls/setgroups 
> -I/home/stas/ltp/testcases/kernel/syscalls/setgroups/../utils 
> -I../../../../include -I../../../../include 
> -DTST_USE_COMPAT16_SYSCALL=1  -c -o setgroups01_16.o setgroups01.c
> gcc   -L../../../../lib  setgroups01_16.o 
> /home/stas/ltp/testcases/kernel/syscalls/setgroups/../utils/compat_16.h 
> -lltp -o setgroups01_16
> 
> It doesn't look nice :)

Ah, it gets to the list of prerequisities and gets apended to the
linking. I Should have noticed this.

I guess that your approach is better.

-- 
Cyril Hrubis
chrubis@suse.cz

------------------------------------------------------------------------------
Learn the latest--Visual Studio 2012, SharePoint 2013, SQL 2012, more!
Discover the easy way to master current and previous Microsoft technologies
and advance your career. Get an incredible 1,500+ hours of step-by-step
tutorial videos with LearnDevNow. Subscribe today and save!
http://pubads.g.doubleclick.net/gampad/clk?id=58040911&iu=/4140/ostg.clktrk
_______________________________________________
Ltp-list mailing list
Ltp-list@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/ltp-list

Re: [LTP] [PATCH V3.1 2/2] fixes for 16-bit syscalls testcases

[chrubis]

Hi!
> diff --git a/testcases/kernel/syscalls/setgroups/setgroups01.c b/testcases/kernel/syscalls/setgroups/setgroups01.c
> index 9d70612..4c7dceb 100644
> --- a/testcases/kernel/syscalls/setgroups/setgroups01.c
> +++ b/testcases/kernel/syscalls/setgroups/setgroups01.c
> @@ -162,6 +162,9 @@ int main(int ac, char **av)
>  
>  		/* check return code */
>  		if (TEST_RETURN == -1) {
> +			if (TEST_ERRNO == ENOSYS)
> +				LTP_NO_COMPAT16_SYSCALL(setgroups, cleanup);

I'm a bit afraid to be so specific here. Currently we should get to this
branch only if there is no compat syscall defined (as far as I can tell)
but that may change.

I would settle for more vague "Syscall %s() not implemented". The fact
that it's the compat version should be known from the test name.

Or alternatively we can add callback parameter to the ALL CAPS functions
and make sure that the particular error is printed only when compat
syscall is called.

> +/*
> + * Copyright (c) Red Hat Inc., 2008

You should probably add your copyright here, as the content of the file
was changed

> + * This program is free software;  you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License as published by
> + * the Free Software Foundation; either version 2 of the License, or
> + * (at your option) any later version.
> + *
> + * This program is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY;  without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See
> + * the GNU General Public License for more details.
> + *
> + * You should have received a copy of the GNU General Public License
> + * along with this program;  if not, write to the Free Software
> + * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA
> + */
> +
> +/* Author: Masatake YAMATO <yamato@redhat.com> */
> +
> +#ifndef __LTP_COMPAT_16_H__
> +#define __LTP_COMPAT_16_H__
> +
> +#include <errno.h>
> +#include "compat_gid.h"
> +#include "linux_syscall_numbers.h"
> +
> +/* If the platform has __NR_scall32 defined it
> + * means that __NR_scall is a 16-bit version of
> + * scall() syscall
      ^
      should probably be sys_name() now
> + */
> +#ifdef TST_USE_COMPAT16_SYSCALL
> +# define LTP_CREATE_SYSCALL(sys_name, ...) \
> +	if (__NR_##sys_name##32 != __LTP__NR_INVALID_SYSCALL) { \
> +		return ltp_syscall(__NR_##sys_name, ##__VA_ARGS__); \
> +	} else { \
> +		errno = ENOSYS; \
> +		return -1; \
> +	}
> +#else
> +# define LTP_CREATE_SYSCALL(sys_name, ...) \
> +	return sys_name(__VA_ARGS__)
> +#endif
> +
> +#define LTP_NO_COMPAT16_SYSCALL(sys_name, cleanup) \
> +	tst_brkm(TCONF, cleanup,\
> +		"16-bit version of %s() is not supported on your platform", \
> +		#sys_name)
> +
> +int SETGROUPS(size_t gidsetsize, GID_T *list)
> +{
> +	LTP_CREATE_SYSCALL(setgroups, gidsetsize, list);
> +}
> +
> +int GETGROUPS(size_t gidsetsize, GID_T *list)
> +{
> +	LTP_CREATE_SYSCALL(getgroups, gidsetsize, list);
> +}
> +
> +#endif /* __LTP_COMPAT_16_H__ */
> +

Generally this version looks fine to me.

-- 
Cyril Hrubis
chrubis@suse.cz

------------------------------------------------------------------------------
Learn the latest--Visual Studio 2012, SharePoint 2013, SQL 2012, more!
Discover the easy way to master current and previous Microsoft technologies
and advance your career. Get an incredible 1,500+ hours of step-by-step
tutorial videos with LearnDevNow. Subscribe today and save!
http://pubads.g.doubleclick.net/gampad/clk?id=58040911&iu=/4140/ostg.clktrk
_______________________________________________
Ltp-list mailing list
Ltp-list@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/ltp-list

Re: [LTP] [PATCH V4] 16-bit syscalls fixes

[chrubis]

Hi!
> Changes since V3:
> 
> [PATCH 1/3]: no changes
> 
> [PATCH 2/3]:
> * Added Oracle copyright string to syscalls/utils/compat_16.h
> * Fixed comment next to LTP_CREATE_SYSCALL
> * Used alternative variant: passing cleanup callback parameter to all
>   caps functions. I think It looks more compact.
> 
> [PATCH 3/3]: Modified invocation of SETGROUPS/GETGROUPS because of changes in [PATCH 2/3]

This version looks good to me.

(Let's wait a day or two so that other have chance to look at the code,
 then, if nobody objects, I will merge it.)

-- 
Cyril Hrubis
chrubis@suse.cz

------------------------------------------------------------------------------
Learn the latest--Visual Studio 2012, SharePoint 2013, SQL 2012, more!
Discover the easy way to master current and previous Microsoft technologies
and advance your career. Get an incredible 1,500+ hours of step-by-step
tutorial videos with LearnDevNow. Subscribe today and save!
http://pubads.g.doubleclick.net/gampad/clk?id=58041391&iu=/4140/ostg.clktrk
_______________________________________________
Ltp-list mailing list
Ltp-list@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/ltp-list

Re: [LTP] [PATCH V4] 16-bit syscalls fixes

[chrubis]

Hi!
> Changes since V3:
> 
> [PATCH 1/3]: no changes
> 
> [PATCH 2/3]:
> * Added Oracle copyright string to syscalls/utils/compat_16.h
> * Fixed comment next to LTP_CREATE_SYSCALL
> * Used alternative variant: passing cleanup callback parameter to all
>   caps functions. I think It looks more compact.
> 
> [PATCH 3/3]: Modified invocation of SETGROUPS/GETGROUPS because of changes in [PATCH 2/3]

Pushed, thanks.

-- 
Cyril Hrubis
chrubis@suse.cz

------------------------------------------------------------------------------
Learn the latest--Visual Studio 2012, SharePoint 2013, SQL 2012, more!
Discover the easy way to master current and previous Microsoft technologies
and advance your career. Get an incredible 1,500+ hours of step-by-step
tutorial videos with LearnDevNow. Subscribe today and save!
http://pubads.g.doubleclick.net/gampad/clk?id=58041391&iu=/4140/ostg.clktrk
_______________________________________________
Ltp-list mailing list
Ltp-list@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/ltp-list