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
_______________________________________________
[EMAIL PROTECTED] mailing list
http://lists.freebsd.org/mailman/listinfo/freebsd-current
To unsubscribe, send any mail to "[EMAIL PROTECTED]"
