From owner-freebsd-current@FreeBSD.ORG Tue Nov 18 08:46:29 2003
Return-Path: <owner-freebsd-current@FreeBSD.ORG>
Delivered-To: freebsd-current@freebsd.org
Received: from mx1.FreeBSD.org (mx1.freebsd.org [216.136.204.125])
by hub.freebsd.org (Postfix) with ESMTP id F024F16A4CE
for <freebsd-current@freebsd.org>;
Tue, 18 Nov 2003 08:46:29 -0800 (PST)
Received: from kazi.fit.vutbr.cz (kazi.fit.vutbr.cz [147.229.8.12])
by mx1.FreeBSD.org (Postfix) with ESMTP id 29DFA43F75
for <freebsd-current@freebsd.org>;
Tue, 18 Nov 2003 08:46:28 -0800 (PST)
(envelope-from cejkar@fit.vutbr.cz)
Received: from kazi.fit.vutbr.cz (localhost [127.0.0.1])
by kazi.fit.vutbr.cz (8.12.10/8.12.9) with ESMTP id hAIGkJUC095549
(version=TLSv1/SSLv3 cipher=EDH-RSA-DES-CBC3-SHA bits=168 verify=NO);
Tue, 18 Nov 2003 17:46:19 +0100 (CET)
Received: (from cejkar@localhost)
by kazi.fit.vutbr.cz (8.12.10/8.12.5/Submit) id hAIGkION095547;
Tue, 18 Nov 2003 17:46:18 +0100 (CET)
X-Authentication-Warning: kazi.fit.vutbr.cz: cejkar set sender to
cejkar@fit.vutbr.cz using -f
Date: Tue, 18 Nov 2003 17:46:18 +0100
From: Rudolf Cejka <cejkar@fit.vutbr.cz>
To: Kirk McKusick <mckusick@beastie.mckusick.com>
Message-ID: <20031118164618.GA84637@fit.vutbr.cz>
References: <200311142119.hAELJPaG011962@beastie.mckusick.com>
Mime-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <200311142119.hAELJPaG011962@beastie.mckusick.com>
User-Agent: Mutt/1.4.1i
X-Scanned-By: MIMEDefang 2.16 (www . roaringpenguin . com / mimedefang)
cc: freebsd-current@freebsd.org
Subject: Re: HEADS-UP new statfs structure
X-BeenThere: freebsd-current@freebsd.org
X-Mailman-Version: 2.1.1
Precedence: list
List-Id: Discussions about the use of FreeBSD-current
<freebsd-current.freebsd.org>
List-Unsubscribe: <http://lists.freebsd.org/mailman/listinfo/freebsd-current>,
<mailto:freebsd-current-request@freebsd.org?subject=unsubscribe>
List-Archive: <http://lists.freebsd.org/pipermail/freebsd-current>
List-Post: <mailto:freebsd-current@freebsd.org>
List-Help: <mailto:freebsd-current-request@freebsd.org?subject=help>
List-Subscribe: <http://lists.freebsd.org/mailman/listinfo/freebsd-current>,
<mailto:freebsd-current-request@freebsd.org?subject=subscribe>
X-List-Received-Date: Tue, 18 Nov 2003 16:46:30 -0000
Kirk McKusick wrote (2003/11/14):
> This is why we make this change now so that it will be in place
> for the masses when 5.2 is released :-)
Hello, and is it possible to review some my (one's from masses :o)
questions/suggestions?
* cvtstatfs() for freebsd4_* compat syscalls does not copy text fields
correctly, so old binaries with new kernel know just about first
16 characters from mount points - what do you think about the
following patch? (Or maybe with even safer sizeof() - but I did not
test it.)
--- sys/kern/vfs_syscalls.c.orig Sun Nov 16 11:12:09 2003
+++ sys/kern/vfs_syscalls.c Sun Nov 16 11:56:07 2003
@@ -645,11 +645,11 @@
osp->f_syncreads = MIN(nsp->f_syncreads, LONG_MAX);
osp->f_asyncreads = MIN(nsp->f_asyncreads, LONG_MAX);
bcopy(nsp->f_fstypename, osp->f_fstypename,
- MIN(MFSNAMELEN, OMNAMELEN));
+ MIN(MFSNAMELEN, OMFSNAMELEN - 1));
bcopy(nsp->f_mntonname, osp->f_mntonname,
- MIN(MFSNAMELEN, OMNAMELEN));
+ MIN(MNAMELEN, OMNAMELEN - 1));
bcopy(nsp->f_mntfromname, osp->f_mntfromname,
- MIN(MFSNAMELEN, OMNAMELEN));
+ MIN(MNAMELEN, OMNAMELEN - 1));
if (suser(td)) {
osp->f_fsid.val[0] = osp->f_fsid.val[1] = 0;
} else {
---
* sys/compat/freebsd32/freebsd32_misc.c: If you look into copy_statfs(),
you copy 88-byte strings into just 80-byte strings. Fortunately it seems
that there are just overwritten spare fields and f_syncreads/f_asyncreads
before they are set to the correct value. What about these patches, which
furthermore are resistant to possible MFSNAMELEN change in the future?
[I'm sorry, these patches are untested]
--- sys/compat/freebsd32/freebsd32.h.orig Tue Nov 18 16:58:28 2003
+++ sys/compat/freebsd32/freebsd32.h Tue Nov 18 16:59:36 2003
@@ -75,6 +75,7 @@
int32_t ru_nivcsw;
};
+#define FREEBSD32_MFSNAMELEN 16 /* length of type name including null */
#define FREEBSD32_MNAMELEN (88 - 2 * sizeof(int32_t)) /* size of on/from name bufs */
struct statfs32 {
@@ -92,7 +93,7 @@
int32_t f_flags;
int32_t f_syncwrites;
int32_t f_asyncwrites;
- char f_fstypename[MFSNAMELEN];
+ char f_fstypename[FREEBSD32_MFSNAMELEN];
char f_mntonname[FREEBSD32_MNAMELEN];
int32_t f_syncreads;
int32_t f_asyncreads;
--- sys/compat/freebsd32/freebsd32_misc.c.orig Tue Nov 18 16:59:49 2003
+++ sys/compat/freebsd32/freebsd32_misc.c Tue Nov 18 17:03:31 2003
@@ -276,6 +276,7 @@
static void
copy_statfs(struct statfs *in, struct statfs32 *out)
{
+ bzero(out, sizeof *out);
CP(*in, *out, f_bsize);
CP(*in, *out, f_iosize);
CP(*in, *out, f_blocks);
@@ -290,14 +291,14 @@
CP(*in, *out, f_flags);
CP(*in, *out, f_syncwrites);
CP(*in, *out, f_asyncwrites);
- bcopy(in->f_fstypename,
- out->f_fstypename, MFSNAMELEN);
- bcopy(in->f_mntonname,
- out->f_mntonname, MNAMELEN);
+ bcopy(in->f_fstypename, out->f_fstypename,
+ MIN(MFSNAMELEN, FREEBSD32_MFSNAMELEN - 1));
+ bcopy(in->f_mntonname, out->f_mntonname,
+ MIN(MNAMELEN, FREEBSD32_MNAMELEN - 1));
CP(*in, *out, f_syncreads);
CP(*in, *out, f_asyncreads);
- bcopy(in->f_mntfromname,
- out->f_mntfromname, MNAMELEN);
+ bcopy(in->f_mntfromname, out->f_mntfromname,
+ MIN(MNAMELEN, FREEBSD32_MNAMELEN - 1));
}
int
---
* sys/ia64/ia32/ia32.h: It seems to me that statfs32 has similar problems
as freebsd32.h - however in this case real and bigger, because statfs32
is now a combination of old and new statfs: old 32bit fields with new
string lengths, so sizeof(statfs32) is changed from 256 to 272. Is this
really correct? If I understand it correctly, it breaks binary
compatibility for old ia32 binaries. I think that MFSNAMELEN/MNAMELEN
should be renamed to OMFSNAMELEN/OMNAMELEN, or ia32.h should define own
IA32_MFSNAMELEN/IA32_MNAMELEN, similarly to freebsd32.h - which is more
secure with respect to further updates in the future.
* sys/ia64/ia32/ia32_misc.c: If ia32.h is changed/fixed, copy_statfs()
has the same problems, as is in freebsd32_misc.c.
* sys/alpha/osf1/osf1_mount.c: And just small trash at the end, because
it is in #ifdef notanymore ... #endif ;o) (however it means, that osf1
statfs calls are completely broken?), but why bsd2osf_statfs() has
3 x max()? What about following patch?
--- sys/alpha/osf1/osf1_mount.c.orig Tue Nov 18 17:40:08 2003
+++ sys/alpha/osf1/osf1_mount.c Tue Nov 18 17:40:52 2003
@@ -88,7 +88,7 @@
{
#ifdef notanymore
-bzero(osfs, sizeof (struct osf1_statfs));
+ bzero(osfs, sizeof (struct osf1_statfs));
if (!strncmp(fsnames[MOUNT_UFS], bsfs->f_fstypename, MFSNAMELEN))
osfs->f_type = OSF1_MOUNT_UFS;
else if (!strncmp(fsnames[MOUNT_NFS], bsfs->f_fstypename, MFSNAMELEN))
@@ -107,12 +107,12 @@
osfs->f_files = bsfs->f_files;
osfs->f_ffree = bsfs->f_ffree;
bcopy(&bsfs->f_fsid, &osfs->f_fsid,
- max(sizeof bsfs->f_fsid, sizeof osfs->f_fsid));
+ min(sizeof bsfs->f_fsid, sizeof osfs->f_fsid));
/* osfs->f_spare zeroed above */
bcopy(bsfs->f_mntonname, osfs->f_mntonname,
- max(sizeof bsfs->f_mntonname, sizeof osfs->f_mntonname));
+ min(sizeof bsfs->f_mntonname, sizeof osfs->f_mntonname - 1));
bcopy(bsfs->f_mntfromname, osfs->f_mntfromname,
- max(sizeof bsfs->f_mntfromname, sizeof osfs->f_mntfromname));
+ min(sizeof bsfs->f_mntfromname, sizeof osfs->f_mntfromname - 1));
/* XXX osfs->f_xxx should be filled in... */
#endif
}
Best regards.
--
Rudolf Cejka <cejkar at fit.vutbr.cz> http://www.fit.vutbr.cz/~cejkar
Brno University of Technology, Faculty of Information Technology
Bozetechova 2, 612 66 Brno, Czech Republic