2002-04-21 10:37:45

by Urban Widmark

[permalink] [raw]
Subject: [patch] 64bit archs doing incorrect magic for smbfs?

[I hope I got the maintainer list right ... ]

Hello

sparc64, ia64, s390x and ppc64 translate the data part of the mount
syscall for smbfs (and ncpfs). I can see this is done to map different
datatypes of a binary struct (uids/pids/...), although I'm not clear on
exactly when this is needed.

I believe this translation isn't always done correctly. The structs used
have a version field that you are supposed to check to ensure that the
data is in the format you think it is.

For smbfs the data part is now often sent as a normal ascii string (when
using samba 2.2.0+) and should then not be modified at all. ncpfs defines
two different formats (v3 and v4), don't know if both are used.


Untested patch vs 2.4.19-pre7-ac2 below adds version number checks for the
smbfs case (yes, it handles the ascii format too). Similar changes are
needed in 2.5.

Another fix for smbfs could be to just drop the mount conversion code.
This would make the older binary format fail, but that's the long term
plan anyway.

/Urban


diff -urN -X exclude linux-2.4.19-pre7-ac2-orig/arch/ia64/ia32/sys_ia32.c linux-2.4.19-pre7-ac2-smbfs/arch/ia64/ia32/sys_ia32.c
--- linux-2.4.19-pre7-ac2-orig/arch/ia64/ia32/sys_ia32.c Sat Apr 20 22:56:31 2002
+++ linux-2.4.19-pre7-ac2-smbfs/arch/ia64/ia32/sys_ia32.c Sun Apr 21 00:56:43 2002
@@ -3882,12 +3882,15 @@
struct smb_mount_data *s = (struct smb_mount_data *)raw_data;
struct smb_mount_data32 *s32 = (struct smb_mount_data32 *)raw_data;

+ if (s32->version != SMB_MOUNT_OLDVERSION)
+ goto out;
s->version = s32->version;
s->mounted_uid = s32->mounted_uid;
s->uid = s32->uid;
s->gid = s32->gid;
s->file_mode = s32->file_mode;
s->dir_mode = s32->dir_mode;
+out:
return raw_data;
}

diff -urN -X exclude linux-2.4.19-pre7-ac2-orig/arch/ppc64/kernel/sys_ppc32.c linux-2.4.19-pre7-ac2-smbfs/arch/ppc64/kernel/sys_ppc32.c
--- linux-2.4.19-pre7-ac2-orig/arch/ppc64/kernel/sys_ppc32.c Sat Apr 20 22:56:16 2002
+++ linux-2.4.19-pre7-ac2-smbfs/arch/ppc64/kernel/sys_ppc32.c Sun Apr 21 00:58:20 2002
@@ -422,12 +422,15 @@
struct smb_mount_data *s = (struct smb_mount_data *)raw_data;
struct smb_mount_data32 *s32 = (struct smb_mount_data32 *)raw_data;

+ if (s32->version != SMB_MOUNT_OLDVERSION)
+ goto out;
s->version = s32->version;
s->mounted_uid = s32->mounted_uid;
s->uid = s32->uid;
s->gid = s32->gid;
s->file_mode = s32->file_mode;
s->dir_mode = s32->dir_mode;
+out:
return raw_data;
}

diff -urN -X exclude linux-2.4.19-pre7-ac2-orig/arch/s390x/kernel/linux32.c linux-2.4.19-pre7-ac2-smbfs/arch/s390x/kernel/linux32.c
--- linux-2.4.19-pre7-ac2-orig/arch/s390x/kernel/linux32.c Sat Apr 20 22:56:16 2002
+++ linux-2.4.19-pre7-ac2-smbfs/arch/s390x/kernel/linux32.c Sun Apr 21 00:57:49 2002
@@ -1673,12 +1673,15 @@
struct smb_mount_data *s = (struct smb_mount_data *)raw_data;
struct smb_mount_data32 *s32 = (struct smb_mount_data32 *)raw_data;

+ if (s32->version != SMB_MOUNT_OLDVERSION)
+ goto out;
s->version = s32->version;
s->mounted_uid = low2highuid(s32->mounted_uid);
s->uid = low2highuid(s32->uid);
s->gid = low2highgid(s32->gid);
s->file_mode = s32->file_mode;
s->dir_mode = s32->dir_mode;
+out:
return raw_data;
}

diff -urN -X exclude linux-2.4.19-pre7-ac2-orig/arch/sparc64/kernel/sys_sparc32.c linux-2.4.19-pre7-ac2-smbfs/arch/sparc64/kernel/sys_sparc32.c
--- linux-2.4.19-pre7-ac2-orig/arch/sparc64/kernel/sys_sparc32.c Sat Apr 20 22:56:31 2002
+++ linux-2.4.19-pre7-ac2-smbfs/arch/sparc64/kernel/sys_sparc32.c Sun Apr 21 00:54:41 2002
@@ -1657,6 +1657,8 @@
struct smb_mount_data news, *s = &news;
struct smb_mount_data32 *s32 = (struct smb_mount_data32 *)raw_data;

+ if (s32->version != SMB_MOUNT_OLDVERSION)
+ goto out;
s->version = s32->version;
s->mounted_uid = low2highuid(s32->mounted_uid);
s->uid = low2highuid(s32->uid);
@@ -1664,6 +1666,7 @@
s->file_mode = s32->file_mode;
s->dir_mode = s32->dir_mode;
memcpy(raw_data, s, sizeof(struct smb_mount_data));
+out:
return raw_data;
}



2002-04-21 10:40:00

by David Miller

[permalink] [raw]
Subject: Re: [patch] 64bit archs doing incorrect magic for smbfs?

From: Urban Widmark <[email protected]>
Date: Sun, 21 Apr 2002 12:37:04 +0200 (CEST)

Untested patch vs 2.4.19-pre7-ac2 below adds version number checks for the
smbfs case (yes, it handles the ascii format too). Similar changes are
needed in 2.5.

I have no problems with this fix, you know the internals better
than me.

2002-04-22 13:32:33

by Petr Vandrovec

[permalink] [raw]
Subject: Re: [patch] 64bit archs doing incorrect magic for smbfs?

On 21 Apr 02 at 12:37, Urban Widmark wrote:
>
> For smbfs the data part is now often sent as a normal ascii string (when
> using samba 2.2.0+) and should then not be modified at all. ncpfs defines
> two different formats (v3 and v4), don't know if both are used.

If you have recent (2.2.0.18/2.2.0.19) ncpfs, format v4 is used (it allows
for 32bit uid/gid). I have no non-ia32 test environment, so if anybody finds
that ncpfs does not work on any platform, feel free to send patches either
to me or to arch maintainers...

> Untested patch vs 2.4.19-pre7-ac2 below adds version number checks for the
> smbfs case (yes, it handles the ascii format too). Similar changes are
> needed in 2.5.

I plan using ASCII format too, but it is not high enough on my TODO list.
If it will make life for Al or other architectures easier, I'll do it
sooner...
Petr Vandrovec
[email protected]

2002-04-22 19:51:19

by Urban Widmark

[permalink] [raw]
Subject: Re: [patch] 64bit archs doing incorrect magic for smbfs?

On Mon, 22 Apr 2002, Petr Vandrovec wrote:

> If you have recent (2.2.0.18/2.2.0.19) ncpfs, format v4 is used (it allows
> for 32bit uid/gid). I have no non-ia32 test environment, so if anybody finds
> that ncpfs does not work on any platform, feel free to send patches either
> to me or to arch maintainers...

Unless the v3 and v4 structs have the same layout (and they don't,
although I may have developed a sudden cross-eydness [1]), the code can't
possibly work for all ncpmounts. It may be unused on some arch's because
they don't need 32->64 translation.

I suggest you put it on your list but below the ascii-fication, since that
is another (better?) solution to the same problem.

/Urban

[1] http://www.anri.barc.usda.gov/emusnow/stereo/stereo.htm