samba - Mar 2004 - kenrel 2.6.4 patch for fixing warning of smbfs on high gid/uid

Hello,

when i compiled latest linux 2.6.4 kernel source with gcc 3.3.2 on
Linux/x86,
i got a few warnings about varaibles beeing compared against constants
where the range of the variable is so that the expression is always
constant.
The explicit comparison has to do with the code for high-uid and gid sheme.

attached you will find a diff which does eliminate this error message
by introducing an inline function that wont result in the message but
it will result in compareable dense binary code with normal optimisations.

-Alex.

-- 
+++ NEU bei GMX und erstmalig in Deutschland: T?V-gepr?fter Virenschutz +++
100% Virenerkennung nach Wildlist. Infos: http://www.gmx.net/virenschutz

Alexander:

Sorry, I didn't write back to you sooner.

I don't understand the need for this patch unless it is working around a
bug in the compiler.

More likely, the bug is in smbfs passing something other than
typeof(uid_t) to high2lowuid().

I'll take a look and write back.


Thanks,
Chris


On Thu, 11 Mar 2004, Alexander Stohr wrote:
> Hello,
>
> when i compiled latest linux 2.6.4 kernel source with gcc 3.3.2 on
> Linux/x86,
> i got a few warnings about varaibles beeing compared against constants
> where the range of the variable is so that the expression is always
> constant.
> The explicit comparison has to do with the code for high-uid and gid sheme.
>
> attached you will find a diff which does eliminate this error message
> by introducing an inline function that wont result in the message but
> it will result in compareable dense binary code with normal optimisations.
>
> -Alex.

Yes, the use of SET_UID/SET_GID in smbfs is a bug. The current meaning of
SET_UID is:

	convert from 32-bit uid (in the kernel) to 16-bit uid (for user),
	if necessary


The usage in smbfs was to copy a binary mount parameter structure from
user space into kernel. The user space structure may be 16-bit uid or
32-bit uid, but the kernel structure is always 32-bit. Therefore, no
conversion is necessary.

The following patch should get rid of the warning:


--- fs/smbfs/inode.c.orig	2004-03-10 21:55:22.000000000 -0500
+++ fs/smbfs/inode.c	2004-03-22 15:40:25.000000000 -0500
@@ -551,8 +551,11 @@
 	if (ver == SMB_MOUNT_OLDVERSION) {
 		mnt->version = oldmnt->version;

-		SET_UID(mnt->uid, oldmnt->uid);
-		SET_GID(mnt->gid, oldmnt->gid);
+		/* Since uid/gid of -1 has no special meaning to smbfs, just
+		   copy the values as-is instead of bothering with
+		   low2high{u,g}id() */
+		mnt->uid = oldmnt->uid;
+		mnt->gid = oldmnt->gid;

 		mnt->file_mode = (oldmnt->file_mode & S_IRWXUGO) | S_IFREG;
 		mnt->dir_mode = (oldmnt->dir_mode & S_IRWXUGO) | S_IFDIR;


The bug was introduced by a patch from Andi Kleen in December 2003:

	http://linus.bkbits.net:8080/linux-2.5/cset@3f81e988KSG5AkI7YGMi7OGAo-DkJg



Additionally, sparc64 and s390 have a bit of similar unneeded code in
their compatibility layers:

	arch/sparc64/kernel/sys_sparc32.c
	arch/s390/kernel/compat_linux.c

They are using low2highuid() when processing the ncpfs and smbfs mount
data, but it's not really needed. (it's only needed when -1 is
'special')



I've always wanted to clean up the naming of some of these macros for a
long time, but I've just been lazy. I'll try to submit appropriate
patches at some point. (SET_UID is a really bad name, it should be
something conveying the meaning of CONVERT_TO_OLD_USER_UID)



Thanks,

Chris Wing
wingc@engin.umich.edu



On Thu, 11 Mar 2004, Alexander Stohr wrote:
> Hello,
>
> when i compiled latest linux 2.6.4 kernel source with gcc 3.3.2 on
> Linux/x86,
> i got a few warnings about varaibles beeing compared against constants
> where the range of the variable is so that the expression is always
> constant.
> The explicit comparison has to do with the code for high-uid and gid sheme.
>
> attached you will find a diff which does eliminate this error message
> by introducing an inline function that wont result in the message but
> it will result in compareable dense binary code with normal optimisations.
>
> -Alex.