2002-10-09 12:31:33

by Martin Schwidefsky

[permalink] [raw]
Subject: [PATCH] 2.5.41 s390 (8/8): 16 bit uid/gids.

Use common code for 16 bit user/groud id system calls on s390x.

diff -urN linux-2.5.41/arch/s390x/config.in linux-2.5.41-s390/arch/s390x/config.in
--- linux-2.5.41/arch/s390x/config.in Mon Oct 7 20:23:25 2002
+++ linux-2.5.41-s390/arch/s390x/config.in Wed Oct 9 14:01:53 2002
@@ -22,6 +22,7 @@
bool 'Symmetric multi-processing support' CONFIG_SMP
bool 'Kernel support for 31 bit emulation' CONFIG_S390_SUPPORT
if [ "$CONFIG_S390_SUPPORT" = "y" ]; then
+ define_bool CONFIG_UID16_SYSCALLS y
tristate 'Kernel support for 31 bit ELF binaries' CONFIG_BINFMT_ELF32
fi

diff -urN linux-2.5.41/arch/s390x/defconfig linux-2.5.41-s390/arch/s390x/defconfig
--- linux-2.5.41/arch/s390x/defconfig Wed Oct 9 14:01:13 2002
+++ linux-2.5.41-s390/arch/s390x/defconfig Wed Oct 9 14:01:53 2002
@@ -39,6 +39,7 @@
#
CONFIG_SMP=y
CONFIG_S390_SUPPORT=y
+CONFIG_UID16_SYSCALLS=y
CONFIG_BINFMT_ELF32=y

#
diff -urN linux-2.5.41/arch/s390x/kernel/binfmt_elf32.c linux-2.5.41-s390/arch/s390x/kernel/binfmt_elf32.c
--- linux-2.5.41/arch/s390x/kernel/binfmt_elf32.c Mon Oct 7 20:25:20 2002
+++ linux-2.5.41-s390/arch/s390x/kernel/binfmt_elf32.c Wed Oct 9 14:01:53 2002
@@ -96,6 +96,7 @@
} while (0)
#endif

+#define __UID16 /* ugly, but no duplicated code */
#include "linux32.h"

typedef _s390_fp_regs32 elf_fpregset_t;
@@ -160,11 +161,6 @@

#include <linux/highuid.h>

-#undef NEW_TO_OLD_UID
-#undef NEW_TO_OLD_GID
-#define NEW_TO_OLD_UID(uid) ((uid) > 65535) ? (u16)overflowuid : (u16)(uid)
-#define NEW_TO_OLD_GID(gid) ((gid) > 65535) ? (u16)overflowgid : (u16)(gid)
-
#define elf_addr_t u32
/*
#define init_elf_binfmt init_elf32_binfmt
diff -urN linux-2.5.41/arch/s390x/kernel/entry.S linux-2.5.41-s390/arch/s390x/kernel/entry.S
--- linux-2.5.41/arch/s390x/kernel/entry.S Wed Oct 9 14:01:46 2002
+++ linux-2.5.41-s390/arch/s390x/kernel/entry.S Wed Oct 9 14:01:53 2002
@@ -400,7 +400,7 @@
.long SYSCALL(sys_mount,sys32_mount_wrapper)
.long SYSCALL(sys_oldumount,sys32_oldumount_wrapper)
.long SYSCALL(sys_ni_syscall,sys32_setuid16_wrapper) /* old setuid16 syscall*/
- .long SYSCALL(sys_ni_syscall,sys32_getuid16) /* old getuid16 syscall*/
+ .long SYSCALL(sys_ni_syscall,sys_getuid16) /* old getuid16 syscall*/
.long SYSCALL(sys_ni_syscall,sys32_stime_wrapper) /* 25 old stime syscall */
.long SYSCALL(sys_ptrace,sys32_ptrace_wrapper)
.long SYSCALL(sys_alarm,sys32_alarm_wrapper)
@@ -422,11 +422,11 @@
.long SYSCALL(sys_times,sys32_times_wrapper)
.long SYSCALL(sys_ni_syscall,sys_ni_syscall) /* old prof syscall */
.long SYSCALL(sys_brk,sys32_brk_wrapper) /* 45 */
- .long SYSCALL(sys_ni_syscall,sys32_setgid16) /* old setgid16 syscall*/
- .long SYSCALL(sys_ni_syscall,sys32_getgid16) /* old getgid16 syscall*/
+ .long SYSCALL(sys_ni_syscall,sys_setgid16) /* old setgid16 syscall*/
+ .long SYSCALL(sys_ni_syscall,sys_getgid16) /* old getgid16 syscall*/
.long SYSCALL(sys_signal,sys32_signal_wrapper)
- .long SYSCALL(sys_ni_syscall,sys32_geteuid16) /* old geteuid16 syscall */
- .long SYSCALL(sys_ni_syscall,sys32_getegid16) /* old getegid16 syscall */
+ .long SYSCALL(sys_ni_syscall,sys_geteuid16) /* old geteuid16 syscall */
+ .long SYSCALL(sys_ni_syscall,sys_getegid16) /* old getegid16 syscall */
.long SYSCALL(sys_acct,sys32_acct_wrapper)
.long SYSCALL(sys_umount,sys32_umount_wrapper)
.long SYSCALL(sys_ni_syscall,sys_ni_syscall) /* old lock syscall */
diff -urN linux-2.5.41/arch/s390x/kernel/linux32.c linux-2.5.41-s390/arch/s390x/kernel/linux32.c
--- linux-2.5.41/arch/s390x/kernel/linux32.c Wed Oct 9 14:01:13 2002
+++ linux-2.5.41-s390/arch/s390x/kernel/linux32.c Wed Oct 9 14:01:53 2002
@@ -15,6 +15,7 @@
*/


+#define __UID16 /* ugly, but no duplicated code */
#include <linux/config.h>
#include <linux/kernel.h>
#include <linux/sched.h>
@@ -68,181 +69,6 @@

#include "linux32.h"

-extern asmlinkage long sys_chown(const char *, uid_t,gid_t);
-extern asmlinkage long sys_lchown(const char *, uid_t,gid_t);
-extern asmlinkage long sys_fchown(unsigned int, uid_t,gid_t);
-extern asmlinkage long sys_setregid(gid_t, gid_t);
-extern asmlinkage long sys_setgid(gid_t);
-extern asmlinkage long sys_setreuid(uid_t, uid_t);
-extern asmlinkage long sys_setuid(uid_t);
-extern asmlinkage long sys_setresuid(uid_t, uid_t, uid_t);
-extern asmlinkage long sys_setresgid(gid_t, gid_t, gid_t);
-extern asmlinkage long sys_setfsuid(uid_t);
-extern asmlinkage long sys_setfsgid(gid_t);
-
-/* For this source file, we want overflow handling. */
-
-#undef high2lowuid
-#undef high2lowgid
-#undef low2highuid
-#undef low2highgid
-#undef SET_UID16
-#undef SET_GID16
-#undef NEW_TO_OLD_UID
-#undef NEW_TO_OLD_GID
-#undef SET_OLDSTAT_UID
-#undef SET_OLDSTAT_GID
-#undef SET_STAT_UID
-#undef SET_STAT_GID
-
-#define high2lowuid(uid) ((uid) > 65535) ? (u16)overflowuid : (u16)(uid)
-#define high2lowgid(gid) ((gid) > 65535) ? (u16)overflowgid : (u16)(gid)
-#define low2highuid(uid) ((uid) == (u16)-1) ? (uid_t)-1 : (uid_t)(uid)
-#define low2highgid(gid) ((gid) == (u16)-1) ? (gid_t)-1 : (gid_t)(gid)
-#define SET_UID16(var, uid) var = high2lowuid(uid)
-#define SET_GID16(var, gid) var = high2lowgid(gid)
-#define NEW_TO_OLD_UID(uid) high2lowuid(uid)
-#define NEW_TO_OLD_GID(gid) high2lowgid(gid)
-#define SET_OLDSTAT_UID(stat, uid) (stat).st_uid = high2lowuid(uid)
-#define SET_OLDSTAT_GID(stat, gid) (stat).st_gid = high2lowgid(gid)
-#define SET_STAT_UID(stat, uid) (stat).st_uid = high2lowuid(uid)
-#define SET_STAT_GID(stat, gid) (stat).st_gid = high2lowgid(gid)
-
-asmlinkage long sys32_chown16(const char * filename, u16 user, u16 group)
-{
- return sys_chown(filename, low2highuid(user), low2highgid(group));
-}
-
-asmlinkage long sys32_lchown16(const char * filename, u16 user, u16 group)
-{
- return sys_lchown(filename, low2highuid(user), low2highgid(group));
-}
-
-asmlinkage long sys32_fchown16(unsigned int fd, u16 user, u16 group)
-{
- return sys_fchown(fd, low2highuid(user), low2highgid(group));
-}
-
-asmlinkage long sys32_setregid16(u16 rgid, u16 egid)
-{
- return sys_setregid(low2highgid(rgid), low2highgid(egid));
-}
-
-asmlinkage long sys32_setgid16(u16 gid)
-{
- return sys_setgid((gid_t)gid);
-}
-
-asmlinkage long sys32_setreuid16(u16 ruid, u16 euid)
-{
- return sys_setreuid(low2highuid(ruid), low2highuid(euid));
-}
-
-asmlinkage long sys32_setuid16(u16 uid)
-{
- return sys_setuid((uid_t)uid);
-}
-
-asmlinkage long sys32_setresuid16(u16 ruid, u16 euid, u16 suid)
-{
- return sys_setresuid(low2highuid(ruid), low2highuid(euid),
- low2highuid(suid));
-}
-
-asmlinkage long sys32_getresuid16(u16 *ruid, u16 *euid, u16 *suid)
-{
- int retval;
-
- if (!(retval = put_user(high2lowuid(current->uid), ruid)) &&
- !(retval = put_user(high2lowuid(current->euid), euid)))
- retval = put_user(high2lowuid(current->suid), suid);
-
- return retval;
-}
-
-asmlinkage long sys32_setresgid16(u16 rgid, u16 egid, u16 sgid)
-{
- return sys_setresgid(low2highgid(rgid), low2highgid(egid),
- low2highgid(sgid));
-}
-
-asmlinkage long sys32_getresgid16(u16 *rgid, u16 *egid, u16 *sgid)
-{
- int retval;
-
- if (!(retval = put_user(high2lowgid(current->gid), rgid)) &&
- !(retval = put_user(high2lowgid(current->egid), egid)))
- retval = put_user(high2lowgid(current->sgid), sgid);
-
- return retval;
-}
-
-asmlinkage long sys32_setfsuid16(u16 uid)
-{
- return sys_setfsuid((uid_t)uid);
-}
-
-asmlinkage long sys32_setfsgid16(u16 gid)
-{
- return sys_setfsgid((gid_t)gid);
-}
-
-asmlinkage long sys32_getgroups16(int gidsetsize, u16 *grouplist)
-{
- u16 groups[NGROUPS];
- int i,j;
-
- if (gidsetsize < 0)
- return -EINVAL;
- i = current->ngroups;
- if (gidsetsize) {
- if (i > gidsetsize)
- return -EINVAL;
- for(j=0;j<i;j++)
- groups[j] = current->groups[j];
- if (copy_to_user(grouplist, groups, sizeof(u16)*i))
- return -EFAULT;
- }
- return i;
-}
-
-asmlinkage long sys32_setgroups16(int gidsetsize, u16 *grouplist)
-{
- u16 groups[NGROUPS];
- int i;
-
- if (!capable(CAP_SETGID))
- return -EPERM;
- if ((unsigned) gidsetsize > NGROUPS)
- return -EINVAL;
- if (copy_from_user(groups, grouplist, gidsetsize * sizeof(u16)))
- return -EFAULT;
- for (i = 0 ; i < gidsetsize ; i++)
- current->groups[i] = (gid_t)groups[i];
- current->ngroups = gidsetsize;
- return 0;
-}
-
-asmlinkage long sys32_getuid16(void)
-{
- return high2lowuid(current->uid);
-}
-
-asmlinkage long sys32_geteuid16(void)
-{
- return high2lowuid(current->euid);
-}
-
-asmlinkage long sys32_getgid16(void)
-{
- return high2lowgid(current->gid);
-}
-
-asmlinkage long sys32_getegid16(void)
-{
- return high2lowgid(current->egid);
-}
-
/* 32-bit timeval and related flotsam. */

struct timeval32
diff -urN linux-2.5.41/arch/s390x/kernel/wrapper32.S linux-2.5.41-s390/arch/s390x/kernel/wrapper32.S
--- linux-2.5.41/arch/s390x/kernel/wrapper32.S Mon Oct 7 20:24:12 2002
+++ linux-2.5.41-s390/arch/s390x/kernel/wrapper32.S Wed Oct 9 14:01:53 2002
@@ -83,7 +83,7 @@
llgtr %r2,%r2 # const char *
llgfr %r3,%r3 # __kernel_old_uid_emu31_t
llgfr %r4,%r4 # __kernel_old_uid_emu31_t
- jg sys32_lchown16 # branch to system call
+ jg sys_lchown16 # branch to system call

.globl sys32_lseek_wrapper
sys32_lseek_wrapper:
@@ -111,7 +111,7 @@
.globl sys32_setuid16_wrapper
sys32_setuid16_wrapper:
llgfr %r2,%r2 # __kernel_old_uid_emu31_t
- jg sys32_setuid16 # branch to system call
+ jg sys_setuid16 # branch to system call

#sys32_getuid16_wrapper # void

@@ -195,7 +195,7 @@
.globl sys32_setgid16_wrapper
sys32_setgid16_wrapper:
llgfr %r2,%r2 # __kernel_old_gid_emu31_t
- jg sys32_setgid16 # branch to system call
+ jg sys_setgid16 # branch to system call

#sys32_getgid16_wrapper # void

@@ -278,13 +278,13 @@
sys32_setreuid16_wrapper:
llgfr %r2,%r2 # __kernel_old_uid_emu31_t
llgfr %r3,%r3 # __kernel_old_uid_emu31_t
- jg sys32_setreuid16 # branch to system call
+ jg sys_setreuid16 # branch to system call

.globl sys32_setregid16_wrapper
sys32_setregid16_wrapper:
llgfr %r2,%r2 # __kernel_old_gid_emu31_t
llgfr %r3,%r3 # __kernel_old_gid_emu31_t
- jg sys32_setregid16 # branch to system call
+ jg sys_setregid16 # branch to system call

#sys32_sigsuspend_wrapper # done in sigsuspend_glue

@@ -344,13 +344,13 @@
sys32_getgroups16_wrapper:
lgfr %r2,%r2 # int
llgtr %r3,%r3 # __kernel_old_gid_emu31_t *
- jg sys32_getgroups16 # branch to system call
+ jg sys_getgroups16 # branch to system call

.globl sys32_setgroups16_wrapper
sys32_setgroups16_wrapper:
lgfr %r2,%r2 # int
llgtr %r3,%r3 # __kernel_old_gid_emu31_t *
- jg sys32_setgroups16 # branch to system call
+ jg sys_setgroups16 # branch to system call

.globl sys32_symlink_wrapper
sys32_symlink_wrapper:
@@ -425,7 +425,7 @@
llgfr %r2,%r2 # unsigned int
llgtr %r3,%r3 # __kernel_old_uid_emu31_t *
llgtr %r4,%r4 # __kernel_old_gid_emu31_t *
- jg sys32_fchown16 # branch to system call
+ jg sys_fchown16 # branch to system call

.globl sys32_getpriority_wrapper
sys32_getpriority_wrapper:
@@ -625,12 +625,12 @@
.globl sys32_setfsuid16_wrapper
sys32_setfsuid16_wrapper:
llgfr %r2,%r2 # __kernel_old_uid_emu31_t
- jg sys32_setfsuid16 # branch to system call
+ jg sys_setfsuid16 # branch to system call

.globl sys32_setfsgid16_wrapper
sys32_setfsgid16_wrapper:
llgfr %r2,%r2 # __kernel_old_gid_emu31_t
- jg sys32_setfsgid16 # branch to system call
+ jg sys_setfsgid16 # branch to system call

.globl sys32_llseek_wrapper
sys32_llseek_wrapper:
@@ -777,14 +777,14 @@
llgfr %r2,%r2 # __kernel_old_uid_emu31_t
llgfr %r3,%r3 # __kernel_old_uid_emu31_t
llgfr %r4,%r4 # __kernel_old_uid_emu31_t
- jg sys32_setresuid16 # branch to system call
+ jg sys_setresuid16 # branch to system call

.globl sys32_getresuid16_wrapper
sys32_getresuid16_wrapper:
llgtr %r2,%r2 # __kernel_old_uid_emu31_t *
llgtr %r3,%r3 # __kernel_old_uid_emu31_t *
llgtr %r4,%r4 # __kernel_old_uid_emu31_t *
- jg sys32_getresuid16 # branch to system call
+ jg sys_getresuid16 # branch to system call

.globl sys32_query_module_wrapper
sys32_query_module_wrapper:
@@ -814,14 +814,14 @@
llgfr %r2,%r2 # __kernel_old_gid_emu31_t
llgfr %r3,%r3 # __kernel_old_gid_emu31_t
llgfr %r4,%r4 # __kernel_old_gid_emu31_t
- jg sys32_setresgid16 # branch to system call
+ jg sys_setresgid16 # branch to system call

.globl sys32_getresgid16_wrapper
sys32_getresgid16_wrapper:
llgtr %r2,%r2 # __kernel_old_gid_emu31_t *
llgtr %r3,%r3 # __kernel_old_gid_emu31_t *
llgtr %r4,%r4 # __kernel_old_gid_emu31_t *
- jg sys32_getresgid16 # branch to system call
+ jg sys_getresgid16 # branch to system call

.globl sys32_prctl_wrapper
sys32_prctl_wrapper:
@@ -895,7 +895,7 @@
llgtr %r2,%r2 # const char *
llgfr %r3,%r3 # __kernel_old_uid_emu31_t
llgfr %r4,%r4 # __kernel_old_gid_emu31_t
- jg sys32_chown16 # branch to system call
+ jg sys_chown16 # branch to system call

.globl sys32_getcwd_wrapper
sys32_getcwd_wrapper:
diff -urN linux-2.5.41/include/asm-s390x/posix_types.h linux-2.5.41-s390/include/asm-s390x/posix_types.h
--- linux-2.5.41/include/asm-s390x/posix_types.h Mon Oct 7 20:25:19 2002
+++ linux-2.5.41-s390/include/asm-s390x/posix_types.h Wed Oct 9 14:01:53 2002
@@ -37,8 +37,8 @@
typedef unsigned short __kernel_uid16_t;
typedef unsigned short __kernel_gid16_t;

-typedef __kernel_uid_t __kernel_old_uid_t;
-typedef __kernel_gid_t __kernel_old_gid_t;
+typedef __kernel_uid16_t __kernel_old_uid_t;
+typedef __kernel_gid16_t __kernel_old_gid_t;
typedef __kernel_uid_t __kernel_uid32_t;
typedef __kernel_gid_t __kernel_gid32_t;

diff -urN linux-2.5.41/include/linux/highuid.h linux-2.5.41-s390/include/linux/highuid.h
--- linux-2.5.41/include/linux/highuid.h Mon Oct 7 20:24:39 2002
+++ linux-2.5.41-s390/include/linux/highuid.h Wed Oct 9 14:01:53 2002
@@ -10,6 +10,11 @@
* CONFIG_UID16 is defined if the given architecture needs to
* support backwards compatibility for old system calls.
*
+ * CONFIG_UID16_SYSCALLS is defined if the given architecture wants to call
+ * backwards compatible syscalls, but not alter the general behavior of
+ * other things that may call these translation macros. For example 64 bit
+ * architectures that have legacy compatibility syscalls.
+ *
* kernel code should use uid_t and gid_t at all times when dealing with
* kernel-private data.
*
@@ -38,7 +43,7 @@
#define DEFAULT_OVERFLOWUID 65534
#define DEFAULT_OVERFLOWGID 65534

-#ifdef CONFIG_UID16
+#if defined(CONFIG_UID16) || defined(__UID16)

/* prevent uid mod 65536 effect by returning a default value for high UIDs */
#define high2lowuid(uid) ((uid) > 65535 ? (old_uid_t)overflowuid : (old_uid_t)(uid))
diff -urN linux-2.5.41/include/linux/types.h linux-2.5.41-s390/include/linux/types.h
--- linux-2.5.41/include/linux/types.h Mon Oct 7 20:24:02 2002
+++ linux-2.5.41-s390/include/linux/types.h Wed Oct 9 14:01:53 2002
@@ -30,7 +30,7 @@
typedef __kernel_uid16_t uid16_t;
typedef __kernel_gid16_t gid16_t;

-#ifdef CONFIG_UID16
+#if defined(CONFIG_UID16) || defined(__UID16)
/* This is defined by include/asm-{arch}/posix_types.h */
typedef __kernel_old_uid_t old_uid_t;
typedef __kernel_old_gid_t old_gid_t;
diff -urN linux-2.5.41/kernel/Makefile linux-2.5.41-s390/kernel/Makefile
--- linux-2.5.41/kernel/Makefile Mon Oct 7 20:23:34 2002
+++ linux-2.5.41-s390/kernel/Makefile Wed Oct 9 14:01:53 2002
@@ -12,7 +12,12 @@

obj-$(CONFIG_GENERIC_ISA_DMA) += dma.o
obj-$(CONFIG_SMP) += cpu.o
+ifeq ($(CONFIG_UID16),y)
obj-$(CONFIG_UID16) += uid16.o
+else
+obj-$(CONFIG_UID16_SYSCALLS) += uid16.o
+endif
+
obj-$(CONFIG_MODULES) += ksyms.o
obj-$(CONFIG_KALLSYMS) += kallsyms.o
obj-$(CONFIG_PM) += pm.o
diff -urN linux-2.5.41/kernel/uid16.c linux-2.5.41-s390/kernel/uid16.c
--- linux-2.5.41/kernel/uid16.c Mon Oct 7 20:24:38 2002
+++ linux-2.5.41-s390/kernel/uid16.c Wed Oct 9 14:01:53 2002
@@ -3,6 +3,7 @@
* together in the faint hope we can take the out in five years time.
*/

+#define __UID16 /* ugly, but no duplicated code */
#include <linux/mm.h>
#include <linux/utsname.h>
#include <linux/mman.h>


2002-10-09 17:16:34

by Linus Torvalds

[permalink] [raw]
Subject: Re: [PATCH] 2.5.41 s390 (8/8): 16 bit uid/gids.


On Wed, 9 Oct 2002, Martin Schwidefsky wrote:
>
> Use common code for 16 bit user/groud id system calls on s390x.

Please make this use the real CONFIG_UID16_SYSCALLS instead of using a
magic __UID16 thing that is s390x-specific. Then you make everybody who
currently uses CONFIG_UID16 do both CONFIG_UID16 and
CONFIG_UID16_SYSCALLS.

We don't want magic config options like __UID16 that aren't exposed as
config options and make people go "Huh?!".

Linus

2002-10-09 17:33:00

by Tim Hockin

[permalink] [raw]
Subject: Re: [PATCH] 2.5.41 s390 (8/8): 16 bit uid/gids.

> > Use common code for 16 bit user/groud id system calls on s390x.
>
> Please make this use the real CONFIG_UID16_SYSCALLS instead of using a
> magic __UID16 thing that is s390x-specific. Then you make everybody who
> currently uses CONFIG_UID16 do both CONFIG_UID16 and
> CONFIG_UID16_SYSCALLS.
>
> We don't want magic config options like __UID16 that aren't exposed as
> config options and make people go "Huh?!".


Linus, This is actually something I sent to Martin (and DaveM). The __UID16
crap is because s390x and Sparc64 (and others?) do not want the highuid
stuff except in very specific places - namely compat code. Just using
CONFIG_UID16_SYSCALLS has the same bad side-effect as CONFIG_UID16 - all or
nothing. In short, we want to build uid16.o with highuid translations, and
a few other compat objects, but not everything. Ugly.

I wasn't aware Martin had tried to push this one quite yet. Have a
preferred solution? I'd already passed this by DaveM and he said the fun
would be getting it past you :)

Tim

2002-10-09 17:40:06

by Linus Torvalds

[permalink] [raw]
Subject: Re: [PATCH] 2.5.41 s390 (8/8): 16 bit uid/gids.


On Wed, 9 Oct 2002, Tim Hockin wrote:
> Linus, This is actually something I sent to Martin (and DaveM). The __UID16
> crap is because s390x and Sparc64 (and others?) do not want the highuid
> stuff except in very specific places - namely compat code. Just using
> CONFIG_UID16_SYSCALLS has the same bad side-effect as CONFIG_UID16 - all or
> nothing. In short, we want to build uid16.o with highuid translations, and
> a few other compat objects, but not everything. Ugly.

So why don't we just split it up into all the sub-options? So that you
have a sm?rg?sbord of real options to select from..

In other words, that __UID16 thing should be a real CONFIG_XXX option.

Linus


2002-10-09 18:20:09

by Tim Hockin

[permalink] [raw]
Subject: Re: [PATCH] 2.5.41 s390 (8/8): 16 bit uid/gids.

> On Wed, 9 Oct 2002, Tim Hockin wrote:
> > Linus, This is actually something I sent to Martin (and DaveM). The __UID16
> > crap is because s390x and Sparc64 (and others?) do not want the highuid
> > stuff except in very specific places - namely compat code. Just using
> > CONFIG_UID16_SYSCALLS has the same bad side-effect as CONFIG_UID16 - all or
> > nothing. In short, we want to build uid16.o with highuid translations, and
> > a few other compat objects, but not everything. Ugly.
>
> So why don't we just split it up into all the sub-options? So that you
> have a sm?rg?sbord of real options to select from..
>
> In other words, that __UID16 thing should be a real CONFIG_XXX option.

Because Sparc64/s390x/? still need to tell highuid.h to do macro magic for
NEW_TO_OLD_UID() and friends in some places and not others. A CONFIG_XXX
applies all the time to all files.

We can make the few sparc64/s390x sections just redefine the macros they
want in the files in question, if you prefer, but uid16.c is still a user of
highuid.h and needs to define __UID16. If you prefer, __UID16 can be called
DO_HIGHUID_CONVERSIONS.

#define DO_HIGHUID_CONVERSIONS
#include <linux/uid16.h>

Or have a new uid16.h that unconditionally defines the macros. Then
highuid.h can include uid16.h IFF CONFIG_UID16, and uid16.c can include
uid16.h. I see this as MORE problematic, because someone, somewhere will
include uid16.h when they meant highuid.h. Forcing a non CONFIG_UID16 arch
to explicity call out "I want uid16 macro conversion for THIS FILE" seems
safe. Ugly, but safe.

2002-10-09 22:06:02

by Linus Torvalds

[permalink] [raw]
Subject: Re: [PATCH] 2.5.41 s390 (8/8): 16 bit uid/gids.


On Wed, 9 Oct 2002, Tim Hockin wrote:
> >
> > In other words, that __UID16 thing should be a real CONFIG_XXX option.
>
> Because Sparc64/s390x/? still need to tell highuid.h to do macro magic for
> NEW_TO_OLD_UID() and friends in some places and not others. A CONFIG_XXX
> applies all the time to all files.

If __UID16 works, then renaming it to CONFIG_UID16_ONLY _must_ also work.

I don't understand your argument about other architectures. I'm claiming
that __UID16 is a config option, and that it must be renamed to _show_
that it is a config option.

If the renaming results in code that doesn't work, then the code didn't
work in the first place or your cpp is incredibly broken.

WE MUST NOT HAVE CONFIG OPTIONS THAT ARE HIDDEN AND CALLED __UID16! That's
my whole point.

Linus

2002-10-09 22:38:09

by Tim Hockin

[permalink] [raw]
Subject: Re: [PATCH] 2.5.41 s390 (8/8): 16 bit uid/gids.

> > Because Sparc64/s390x/? still need to tell highuid.h to do macro magic for
> > NEW_TO_OLD_UID() and friends in some places and not others. A CONFIG_XXX
> > applies all the time to all files.
>
> If __UID16 works, then renaming it to CONFIG_UID16_ONLY _must_ also work.
>
> I don't understand your argument about other architectures. I'm claiming
> that __UID16 is a config option, and that it must be renamed to _show_
> that it is a config option.

Renaming it, if that is all you want, is fine by everyone, I'm sure. I was
trying to make the point that it is NOT a config option, because for some
architectures, you only want it defined for SOME FILES.

architectures that define CONFIG_UID16 get high2lowuid() all the time
- no changes needed on any of them

architectures that do not define CONFIG_UID16 and don't want it ever get
straight uids, no high2lowuid() cruft
- no changes needed on any of them

architectures that can use uid16.c code, but do not want high2lowuid()
everywhere need to change
- define CONFIG_UID16_SYSCALLS (build uid16.o)
- explicitly call out where it wants high2lowuid()
- this was called __UID16, but can be called
CONFIG_UID16_HERE or REALLY_DO_HIGHUID_CRAP or FRED for all
I care.

I just want to make sure that you're telling me to name something CONFIG_FOO
when, in fact, it is defined by various bits of code that want to change the
behavior of a particular header.

In that case we can just have them define CONFIG_UID16 and not add an OR in
there at all. I just thought you'd find a .c file defining CONFIG_FOO to be
grosser, yet. But on further argument, I guess it's not.

I'll consider it a done deal, unless you want to stop me :) Martin, do you
want to just change the __UID16 defines to CONFIG_UID16 and nix the || in
highuid.h and resubmit?

I have the same patch in my local tree.

Tim

2002-10-09 23:05:01

by Linus Torvalds

[permalink] [raw]
Subject: Re: [PATCH] 2.5.41 s390 (8/8): 16 bit uid/gids.


On Wed, 9 Oct 2002, Tim Hockin wrote:
>
> Renaming it, if that is all you want, is fine by everyone, I'm sure. I was
> trying to make the point that it is NOT a config option, because for some
> architectures, you only want it defined for SOME FILES.

No. I'm not going to accept crap like that.

It's either defined or not. None of the "only sometimes".

Make a CONFIG_xxx option that means what s390 wants it to mean, and no,
I'm not going to accept it if you #define that by hand in some .c file. It
needs to be in the architecture config file, like all the other config
options.

Linus

2002-10-09 23:40:24

by David Miller

[permalink] [raw]
Subject: Re: [PATCH] 2.5.41 s390 (8/8): 16 bit uid/gids.

From: Linus Torvalds <[email protected]>
Date: Wed, 9 Oct 2002 16:12:39 -0700 (PDT)

Make a CONFIG_xxx option that means what s390 wants it to mean, and no,
I'm not going to accept it if you #define that by hand in some .c file. It
needs to be in the architecture config file, like all the other config
options.

I understand what you want.

He's just trying to kill all this duplicate code we have on
s390x/ppc64/sparc64/etc. which does UID16 stuff for the 32-bit
compat case

So maybe what you're saying is some "asm/uid16.h" that
conditionalizes?

2002-10-09 23:53:47

by Tim Hockin

[permalink] [raw]
Subject: Re: [PATCH] 2.5.41 s390 (8/8): 16 bit uid/gids.

> So maybe what you're saying is some "asm/uid16.h" that
> conditionalizes?


It doesn't need to be asm/ - it can all be generic. You just have
<linux/uid16_convert.h> be the unconditional macro definitions.
<linux/highuid.h> can conditionally include that. Only downside is that you
have to define the NOP version of the macros in highuid.h, but it's not like
these are growing or evolving.

I'm fine with that or manually defining CONFIG_UID16 - it is really for
special cases. I just loathe duplicated code :)

Tim