Re: 64-bit inodes (ino64) Status Update and Call for Testing
On Sun, May 21, 2017 at 05:25:35PM +0300, Konstantin Belousov wrote: > On Sun, May 21, 2017 at 04:03:55PM +0200, Jilles Tjoelker wrote: > > On Sun, May 21, 2017 at 03:31:18PM +0300, Konstantin Belousov wrote: > > > On Sun, May 21, 2017 at 02:14:56PM +0200, Jilles Tjoelker wrote: > > > > We have another type in this area which is too small in some situations: > > > > uint8_t for struct dirent.d_namlen. For filesystems that store filenames > > > > as upto 255 UTF-16 code units, the name to be stored in d_name may be > > > > upto 765 bytes long in UTF-8. This was reported in PR 204643. The code > > > > currently handles this by returning the short (8.3) name, but this name > > > > may not be present or usable, leaving the file inaccessible. > > > > Actually allowing longer names seems too complicated to add to the ino64 > > > > change, but changing d_namlen to uint16_t (using d_pad0 space) and > > > > skipping entries with d_namlen > 255 in libc may be helpful. > > > > Note that applications using the deprecated readdir_r() will not be able > > > > to read such long names, since the API does not allow specifying that a > > > > larger buffer has been provided. (This could be avoided by making struct > > > > dirent.d_name 766 bytes long instead of 256.) > > > > Unfortunately, the existence of readdir_r() also prevents changing > > > > struct dirent.d_name to the more correct flexible array. > > > Yes, changing the size of d_name at this stage of the project is out of > > > question. My reading of your proposal is that we should extend the size > > > of d_namlen to uint16_t, am I right ? Should we go to 32bit directly > > > then, perhaps ? > > Yes, my proposal is to change d_namlen to uint16_t. > > Making it 32 bits is not useful with the 16-bit d_reclen, and increasing > > d_reclen does not seem useful to me with the current model of > > getdirentries() where the whole dirent must fit into the caller's > > buffer. > Bumping it now might cause less churn later, even if unused, but ok. > > > I did not committed the change below, nor did I tested or even build it. > > I'd like to skip overlong names in the native readdir_r() as well, so > > that long name support can be added to the kernel later without causing > > buffer overflows with applications using FreeBSD 12.0 libc. > > The native readdir() does not seem to have such a problem. > Again, not even compiled. Looks good to me. > [patch snipped] -- Jilles Tjoelker ___ freebsd-ports@freebsd.org mailing list https://lists.freebsd.org/mailman/listinfo/freebsd-ports To unsubscribe, send any mail to "freebsd-ports-unsubscr...@freebsd.org"
Re: 64-bit inodes (ino64) Status Update and Call for Testing
On Sun, May 21, 2017 at 04:03:55PM +0200, Jilles Tjoelker wrote:
> On Sun, May 21, 2017 at 03:31:18PM +0300, Konstantin Belousov wrote:
> > On Sun, May 21, 2017 at 02:14:56PM +0200, Jilles Tjoelker wrote:
> > > We have another type in this area which is too small in some situations:
> > > uint8_t for struct dirent.d_namlen. For filesystems that store filenames
> > > as upto 255 UTF-16 code units, the name to be stored in d_name may be
> > > upto 765 bytes long in UTF-8. This was reported in PR 204643. The code
> > > currently handles this by returning the short (8.3) name, but this name
> > > may not be present or usable, leaving the file inaccessible.
>
> > > Actually allowing longer names seems too complicated to add to the ino64
> > > change, but changing d_namlen to uint16_t (using d_pad0 space) and
> > > skipping entries with d_namlen > 255 in libc may be helpful.
>
> > > Note that applications using the deprecated readdir_r() will not be able
> > > to read such long names, since the API does not allow specifying that a
> > > larger buffer has been provided. (This could be avoided by making struct
> > > dirent.d_name 766 bytes long instead of 256.)
>
> > > Unfortunately, the existence of readdir_r() also prevents changing
> > > struct dirent.d_name to the more correct flexible array.
>
> > Yes, changing the size of d_name at this stage of the project is out of
> > question. My reading of your proposal is that we should extend the size
> > of d_namlen to uint16_t, am I right ? Should we go to 32bit directly
> > then, perhaps ?
>
> Yes, my proposal is to change d_namlen to uint16_t.
>
> Making it 32 bits is not useful with the 16-bit d_reclen, and increasing
> d_reclen does not seem useful to me with the current model of
> getdirentries() where the whole dirent must fit into the caller's
> buffer.
Bumping it now might cause less churn later, even if unused, but ok.
>
> > I did not committed the change below, nor did I tested or even build it.
>
> I'd like to skip overlong names in the native readdir_r() as well, so
> that long name support can be added to the kernel later without causing
> buffer overflows with applications using FreeBSD 12.0 libc.
>
> The native readdir() does not seem to have such a problem.
Again, not even compiled.
diff --git a/lib/libc/gen/readdir-compat11.c b/lib/libc/gen/readdir-compat11.c
index 1c52f563c75..a865ab9157e 100644
--- a/lib/libc/gen/readdir-compat11.c
+++ b/lib/libc/gen/readdir-compat11.c
@@ -41,6 +41,7 @@ __FBSDID("$FreeBSD$");
#define_WANT_FREEBSD11_DIRENT
#include
#include
+#include
#include
#include
#include
@@ -53,10 +54,12 @@ __FBSDID("$FreeBSD$");
#include "gen-compat.h"
-static void
+static bool
freebsd11_cvtdirent(struct freebsd11_dirent *dstdp, struct dirent *srcdp)
{
+ if (srcdp->d_namelen >= sizeof(dstdp->d_name))
+ return (false);
dstdp->d_type = srcdp->d_type;
dstdp->d_namlen = srcdp->d_namlen;
dstdp->d_fileno = srcdp->d_fileno; /* truncate */
@@ -65,6 +68,7 @@ freebsd11_cvtdirent(struct freebsd11_dirent *dstdp, struct
dirent *srcdp)
bzero(dstdp->d_name + dstdp->d_namlen,
dstdp->d_reclen - offsetof(struct freebsd11_dirent, d_name) -
dstdp->d_namlen);
+ return (true);
}
struct freebsd11_dirent *
@@ -75,13 +79,15 @@ freebsd11_readdir(DIR *dirp)
if (__isthreaded)
_pthread_mutex_lock(>dd_lock);
- dp = _readdir_unlocked(dirp, 1);
+ dp = _readdir_unlocked(dirp, RDU_SKIP);
if (dp != NULL) {
if (dirp->dd_compat_de == NULL)
dirp->dd_compat_de = malloc(sizeof(struct
freebsd11_dirent));
- freebsd11_cvtdirent(dirp->dd_compat_de, dp);
- dstdp = dirp->dd_compat_de;
+ if (freebsd11_cvtdirent(dirp->dd_compat_de, dp))
+ dstdp = dirp->dd_compat_de;
+ else
+ dstdp = NULL;
} else
dstdp = NULL;
if (__isthreaded)
@@ -101,8 +107,10 @@ freebsd11_readdir_r(DIR *dirp, struct freebsd11_dirent
*entry,
if (error != 0)
return (error);
if (xresult != NULL) {
- freebsd11_cvtdirent(entry, );
- *result = entry;
+ if (freebsd11_cvtdirent(entry, ))
+ *result = entry;
+ else /* should not happen due to RDU_SHORT */
+ *result = NULL;
} else
*result = NULL;
return (0);
diff --git a/lib/libc/gen/readdir.c b/lib/libc/gen/readdir.c
index dc7b0df18b2..c30d48273b8 100644
--- a/lib/libc/gen/readdir.c
+++ b/lib/libc/gen/readdir.c
@@ -49,7 +49,7 @@ __FBSDID("$FreeBSD$");
* get next entry in a directory.
*/
struct dirent *
-_readdir_unlocked(DIR *dirp, int skip)
+_readdir_unlocked(DIR *dirp, int flags)
{
struct dirent *dp;
long
Re: 64-bit inodes (ino64) Status Update and Call for Testing
On Sun, May 21, 2017 at 03:31:18PM +0300, Konstantin Belousov wrote: > On Sun, May 21, 2017 at 02:14:56PM +0200, Jilles Tjoelker wrote: > > We have another type in this area which is too small in some situations: > > uint8_t for struct dirent.d_namlen. For filesystems that store filenames > > as upto 255 UTF-16 code units, the name to be stored in d_name may be > > upto 765 bytes long in UTF-8. This was reported in PR 204643. The code > > currently handles this by returning the short (8.3) name, but this name > > may not be present or usable, leaving the file inaccessible. > > Actually allowing longer names seems too complicated to add to the ino64 > > change, but changing d_namlen to uint16_t (using d_pad0 space) and > > skipping entries with d_namlen > 255 in libc may be helpful. > > Note that applications using the deprecated readdir_r() will not be able > > to read such long names, since the API does not allow specifying that a > > larger buffer has been provided. (This could be avoided by making struct > > dirent.d_name 766 bytes long instead of 256.) > > Unfortunately, the existence of readdir_r() also prevents changing > > struct dirent.d_name to the more correct flexible array. > Yes, changing the size of d_name at this stage of the project is out of > question. My reading of your proposal is that we should extend the size > of d_namlen to uint16_t, am I right ? Should we go to 32bit directly > then, perhaps ? Yes, my proposal is to change d_namlen to uint16_t. Making it 32 bits is not useful with the 16-bit d_reclen, and increasing d_reclen does not seem useful to me with the current model of getdirentries() where the whole dirent must fit into the caller's buffer. > I did not committed the change below, nor did I tested or even build it. I'd like to skip overlong names in the native readdir_r() as well, so that long name support can be added to the kernel later without causing buffer overflows with applications using FreeBSD 12.0 libc. The native readdir() does not seem to have such a problem. > [patch snipped] -- Jilles Tjoelker ___ freebsd-ports@freebsd.org mailing list https://lists.freebsd.org/mailman/listinfo/freebsd-ports To unsubscribe, send any mail to "freebsd-ports-unsubscr...@freebsd.org"
Re: 64-bit inodes (ino64) Status Update and Call for Testing
On Sun, May 21, 2017 at 02:14:56PM +0200, Jilles Tjoelker wrote:
> We have another type in this area which is too small in some situations:
> uint8_t for struct dirent.d_namlen. For filesystems that store filenames
> as upto 255 UTF-16 code units, the name to be stored in d_name may be
> upto 765 bytes long in UTF-8. This was reported in PR 204643. The code
> currently handles this by returning the short (8.3) name, but this name
> may not be present or usable, leaving the file inaccessible.
>
> Actually allowing longer names seems too complicated to add to the ino64
> change, but changing d_namlen to uint16_t (using d_pad0 space) and
> skipping entries with d_namlen > 255 in libc may be helpful.
>
> Note that applications using the deprecated readdir_r() will not be able
> to read such long names, since the API does not allow specifying that a
> larger buffer has been provided. (This could be avoided by making struct
> dirent.d_name 766 bytes long instead of 256.)
>
> Unfortunately, the existence of readdir_r() also prevents changing
> struct dirent.d_name to the more correct flexible array.
Yes, changing the size of d_name at this stage of the project is out of
question. My reading of your proposal is that we should extend the size
of d_namlen to uint16_t, am I right ? Should we go to 32bit directly
then, perhaps ?
I did not committed the change below, nor did I tested or even build it.
diff --git a/lib/libc/gen/readdir-compat11.c b/lib/libc/gen/readdir-compat11.c
index 1c52f563c75..18d85adaa63 100644
--- a/lib/libc/gen/readdir-compat11.c
+++ b/lib/libc/gen/readdir-compat11.c
@@ -41,6 +41,7 @@ __FBSDID("$FreeBSD$");
#define_WANT_FREEBSD11_DIRENT
#include
#include
+#include
#include
#include
#include
@@ -53,10 +54,12 @@ __FBSDID("$FreeBSD$");
#include "gen-compat.h"
-static void
+static bool
freebsd11_cvtdirent(struct freebsd11_dirent *dstdp, struct dirent *srcdp)
{
+ if (srcdp->d_namelen >= sizeof(dstdp->d_name))
+ return (false);
dstdp->d_type = srcdp->d_type;
dstdp->d_namlen = srcdp->d_namlen;
dstdp->d_fileno = srcdp->d_fileno; /* truncate */
@@ -65,6 +68,7 @@ freebsd11_cvtdirent(struct freebsd11_dirent *dstdp, struct
dirent *srcdp)
bzero(dstdp->d_name + dstdp->d_namlen,
dstdp->d_reclen - offsetof(struct freebsd11_dirent, d_name) -
dstdp->d_namlen);
+ return (true);
}
struct freebsd11_dirent *
@@ -80,8 +84,10 @@ freebsd11_readdir(DIR *dirp)
if (dirp->dd_compat_de == NULL)
dirp->dd_compat_de = malloc(sizeof(struct
freebsd11_dirent));
- freebsd11_cvtdirent(dirp->dd_compat_de, dp);
- dstdp = dirp->dd_compat_de;
+ if (freebsd11_cvtdirent(dirp->dd_compat_de, dp))
+ dstdp = dirp->dd_compat_de;
+ else
+ dstdp = NULL;
} else
dstdp = NULL;
if (__isthreaded)
@@ -101,8 +107,10 @@ freebsd11_readdir_r(DIR *dirp, struct freebsd11_dirent
*entry,
if (error != 0)
return (error);
if (xresult != NULL) {
- freebsd11_cvtdirent(entry, );
- *result = entry;
+ if (freebsd11_cvtdirent(entry, ))
+ *result = entry;
+ else
+ *result = NULL;
} else
*result = NULL;
return (0);
diff --git a/sys/kern/vfs_syscalls.c b/sys/kern/vfs_syscalls.c
index 784af836aee..27b2635030d 100644
--- a/sys/kern/vfs_syscalls.c
+++ b/sys/kern/vfs_syscalls.c
@@ -3733,7 +3733,8 @@ freebsd11_kern_getdirentries(struct thread *td, int fd,
char *ubuf, u_int count,
if (dp->d_reclen == 0)
break;
MPASS(dp->d_reclen >= _GENERIC_DIRLEN(0));
- /* dp->d_namlen <= sizeof(dstdp.d_name) - 1 always */
+ if (dp->d_namlen >= sizeof(dstdp.d_name))
+ continue;
dstdp.d_type = dp->d_type;
dstdp.d_namlen = dp->d_namlen;
dstdp.d_fileno = dp->d_fileno; /* truncate */
diff --git a/sys/sys/dirent.h b/sys/sys/dirent.h
index 341855d0530..691c4e8f90f 100644
--- a/sys/sys/dirent.h
+++ b/sys/sys/dirent.h
@@ -67,8 +67,9 @@ struct dirent {
off_t d_off; /* directory offset of entry */
__uint16_t d_reclen;/* length of this record */
__uint8_t d_type; /* file type, see below */
- __uint8_t d_namlen;/* length of string in d_name */
- __uint32_t d_pad0;
+ __uint8_t d_pad0
+ __uint16_t d_namlen;/* length of string in d_name */
+ __uint16_t d_pad1;
#if __BSD_VISIBLE
#defineMAXNAMLEN 255
chard_name[MAXNAMLEN + 1]; /* name must be no longer than this */
Re: 64-bit inodes (ino64) Status Update and Call for Testing
On Thu, Apr 20, 2017 at 10:43:14PM +0300, Konstantin Belousov wrote: > Inodes are data structures corresponding to objects in a file system, > such as files and directories. FreeBSD has historically used 32-bit > values to identify inodes, which limits file systems to somewhat under > 2^32 objects. Many modern file systems internally use 64-bit identifiers > and FreeBSD needs to follow suit to properly and fully support these > file systems. > The 64-bit inode project, also known as ino64, started life many years > ago as a project by Gleb Kurtsou (gleb@). After that time several > people have had a hand in updating it and addressing regressions, after > mckusick@ picked up and updated the patch, and acted as a flag-waver. > Sponsored by the FreeBSD Foundation I have spent a significant effort > on outstanding issues and integration -- fixing compat32 ABI, NFS and > ZFS, addressing ABI compat issues and investigating and fixing ports > failures. rmacklem@ provided feedback on NFS changes, emaste@ and > jhb@ provided feedback and review on the ABI transition support. pho@ > performed extensive testing and identified a number of issues that > have now been fixed. kris@ performed an initial ports investigation > followed by an exp-run by antoine@. emaste@ helped with organization > of the process. > This note explains how to perform useful testing of the ino64 branch, > beyond typical smoke tests. > 1. Overview. > The ino64 branch extends the basic system types ino_t and dev_t from > 32-bit to 64-bit, and nlink_t from 16-bit to 64-bit. The struct dirent > layout is modified due to the larger size of ino_t, and also gains a > d_off (directory offset) member. As ino64 implies an ABI change anyway > the struct statfs f_mntfromname[] and f_mntonname[] array length > MNAMELEN is increased from 88 to 1024, to allow for longer mount path > names. > ABI breakage is mitigated by providing compatibility using versioned > symbols, ingenious use of the existing padding in structures, and by > employing other tricks. Unfortunately, not everything can be fixed, > especially outside the base system. For instance, third-party APIs > which pass struct stat around are broken in backward and forward- > incompatible way. We have another type in this area which is too small in some situations: uint8_t for struct dirent.d_namlen. For filesystems that store filenames as upto 255 UTF-16 code units, the name to be stored in d_name may be upto 765 bytes long in UTF-8. This was reported in PR 204643. The code currently handles this by returning the short (8.3) name, but this name may not be present or usable, leaving the file inaccessible. Actually allowing longer names seems too complicated to add to the ino64 change, but changing d_namlen to uint16_t (using d_pad0 space) and skipping entries with d_namlen > 255 in libc may be helpful. Note that applications using the deprecated readdir_r() will not be able to read such long names, since the API does not allow specifying that a larger buffer has been provided. (This could be avoided by making struct dirent.d_name 766 bytes long instead of 256.) Unfortunately, the existence of readdir_r() also prevents changing struct dirent.d_name to the more correct flexible array. -- Jilles Tjoelker ___ freebsd-ports@freebsd.org mailing list https://lists.freebsd.org/mailman/listinfo/freebsd-ports To unsubscribe, send any mail to "freebsd-ports-unsubscr...@freebsd.org"
Re: 64-bit inodes (ino64) Status Update and Call for Testing
On Mon, May 15, 2017 at 03:41:45PM -0400, Shawn Webb wrote: > On Thu, Apr 20, 2017 at 10:43:14PM +0300, Konstantin Belousov wrote: > > Inodes are data structures corresponding to objects in a file system, > > such as files and directories. FreeBSD has historically used 32-bit > > values to identify inodes, which limits file systems to somewhat under > > 2^32 objects. Many modern file systems internally use 64-bit identifiers > > and FreeBSD needs to follow suit to properly and fully support these > > file systems. > > > > The 64-bit inode project, also known as ino64, started life many years > > ago as a project by Gleb Kurtsou (gleb@). After that time several > > people have had a hand in updating it and addressing regressions, after > > mckusick@ picked up and updated the patch, and acted as a flag-waver. > > > > Sponsored by the FreeBSD Foundation I have spent a significant effort > > on outstanding issues and integration -- fixing compat32 ABI, NFS and > > ZFS, addressing ABI compat issues and investigating and fixing ports > > failures. rmacklem@ provided feedback on NFS changes, emaste@ and > > jhb@ provided feedback and review on the ABI transition support. pho@ > > performed extensive testing and identified a number of issues that > > have now been fixed. kris@ performed an initial ports investigation > > followed by an exp-run by antoine@. emaste@ helped with organization > > of the process. > > > > This note explains how to perform useful testing of the ino64 branch, > > beyond typical smoke tests. > > > > 1. Overview. > > > > The ino64 branch extends the basic system types ino_t and dev_t from > > 32-bit to 64-bit, and nlink_t from 16-bit to 64-bit. The struct dirent > > layout is modified due to the larger size of ino_t, and also gains a > > d_off (directory offset) member. As ino64 implies an ABI change anyway > > the struct statfs f_mntfromname[] and f_mntonname[] array length > > MNAMELEN is increased from 88 to 1024, to allow for longer mount path > > names. > > > > ABI breakage is mitigated by providing compatibility using versioned > > symbols, ingenious use of the existing padding in structures, and by > > employing other tricks. Unfortunately, not everything can be fixed, > > especially outside the base system. For instance, third-party APIs > > which pass struct stat around are broken in backward and forward- > > incompatible way. > > > > 2. Motivation. > > > > The main risk of the ino64 change is the uncontrolled ABI breakage. > > Due to expansion of the basic types dev_t, ino_t and struct dirent, > > the impact is not limited to one part of the system, but affects: > > - kernel/userspace interface (syscalls ABI, mostly stat(2), kinfo > > and more) > > - libc interface (mostly related to the readdir(3), FTS(3)) > > - collateral damage in other libraries that happens to use changed types > > in the interfaces. See, for instance, libprocstat, for which compat > > was provided using symbol versioning, and libutil, which shlib version > > was bumped. > > > > 3. Quirks. > > > > We handled kinfo sysctl MIBs, but other MIBs which report structures > > depended on the changed type, are not handled in general. It was > > considered that the breakage is either in the management interfaces, > > where we usually allow ABI slip, or is not important. > > > > Struct xvnode changed layout, no compat shims are provided. > > > > For struct xtty, dev_t tty device member was reduced to uint32_t. It > > was decided that keeping ABI compat in this case is more useful than > > reporting 64bit dev_t, for the sake of pstat. > > > > 4. Testing procedure. > > > > The ino64 project can be tested by cloning the project branch from > > GitHub or by applying the patch > at URL | attached> to a working tree. The authorative source is the > > GitHub, I do not promise to update the review for each update. > > > > To clone from GitHub: > > % git clone -b ino64 https://github.com/FreeBSDFoundation/freebsd.git ino64 > > > > To fetch the patch from Phabricator: > > - Visit https://reviews.freebsd.org/D10439 > > - Click "Download Raw Diff" at the upper right of the page > > > > Or > > % arc patch D10439 > > > > After that, in the checkout directory do > > % (cd sys/kern && touch syscalls.master && make sysent) > > % (cd sys/compat/freebsd32 && touch syscalls.master && make sysent) > > If you use custom kernel configuration, ensure that > > options COMPAT_FREEBSD11 > > is included into the config. Then build world and kernel in the > > usual way, install kernel, reboot, install new world. Do not make > > shortcuts in the update procedure. > > Hey Kostik, > > On my HardenedBSD system, world compiled fine with the patch, but the > kernel didn't compile. I've uploaded the log to: > > https://hardenedbsd.org/~shawn/2017-05-15_ino64-r01.log > > This was from importing the patch from Phabricator into > hardened/current/master in HardenedBSD. Update: I missed a step. Kernel and
Re: 64-bit inodes (ino64) Status Update and Call for Testing
On Thu, Apr 20, 2017 at 10:43:14PM +0300, Konstantin Belousov wrote: > Inodes are data structures corresponding to objects in a file system, > such as files and directories. FreeBSD has historically used 32-bit > values to identify inodes, which limits file systems to somewhat under > 2^32 objects. Many modern file systems internally use 64-bit identifiers > and FreeBSD needs to follow suit to properly and fully support these > file systems. > > The 64-bit inode project, also known as ino64, started life many years > ago as a project by Gleb Kurtsou (gleb@). After that time several > people have had a hand in updating it and addressing regressions, after > mckusick@ picked up and updated the patch, and acted as a flag-waver. > > Sponsored by the FreeBSD Foundation I have spent a significant effort > on outstanding issues and integration -- fixing compat32 ABI, NFS and > ZFS, addressing ABI compat issues and investigating and fixing ports > failures. rmacklem@ provided feedback on NFS changes, emaste@ and > jhb@ provided feedback and review on the ABI transition support. pho@ > performed extensive testing and identified a number of issues that > have now been fixed. kris@ performed an initial ports investigation > followed by an exp-run by antoine@. emaste@ helped with organization > of the process. > > This note explains how to perform useful testing of the ino64 branch, > beyond typical smoke tests. > > 1. Overview. > > The ino64 branch extends the basic system types ino_t and dev_t from > 32-bit to 64-bit, and nlink_t from 16-bit to 64-bit. The struct dirent > layout is modified due to the larger size of ino_t, and also gains a > d_off (directory offset) member. As ino64 implies an ABI change anyway > the struct statfs f_mntfromname[] and f_mntonname[] array length > MNAMELEN is increased from 88 to 1024, to allow for longer mount path > names. > > ABI breakage is mitigated by providing compatibility using versioned > symbols, ingenious use of the existing padding in structures, and by > employing other tricks. Unfortunately, not everything can be fixed, > especially outside the base system. For instance, third-party APIs > which pass struct stat around are broken in backward and forward- > incompatible way. > > 2. Motivation. > > The main risk of the ino64 change is the uncontrolled ABI breakage. > Due to expansion of the basic types dev_t, ino_t and struct dirent, > the impact is not limited to one part of the system, but affects: > - kernel/userspace interface (syscalls ABI, mostly stat(2), kinfo > and more) > - libc interface (mostly related to the readdir(3), FTS(3)) > - collateral damage in other libraries that happens to use changed types > in the interfaces. See, for instance, libprocstat, for which compat > was provided using symbol versioning, and libutil, which shlib version > was bumped. > > 3. Quirks. > > We handled kinfo sysctl MIBs, but other MIBs which report structures > depended on the changed type, are not handled in general. It was > considered that the breakage is either in the management interfaces, > where we usually allow ABI slip, or is not important. > > Struct xvnode changed layout, no compat shims are provided. > > For struct xtty, dev_t tty device member was reduced to uint32_t. It > was decided that keeping ABI compat in this case is more useful than > reporting 64bit dev_t, for the sake of pstat. > > 4. Testing procedure. > > The ino64 project can be tested by cloning the project branch from > GitHub or by applying the patch at URL | attached> to a working tree. The authorative source is the > GitHub, I do not promise to update the review for each update. > > To clone from GitHub: > % git clone -b ino64 https://github.com/FreeBSDFoundation/freebsd.git ino64 > > To fetch the patch from Phabricator: > - Visit https://reviews.freebsd.org/D10439 > - Click "Download Raw Diff" at the upper right of the page > > Or > % arc patch D10439 > > After that, in the checkout directory do > % (cd sys/kern && touch syscalls.master && make sysent) > % (cd sys/compat/freebsd32 && touch syscalls.master && make sysent) > If you use custom kernel configuration, ensure that > options COMPAT_FREEBSD11 > is included into the config. Then build world and kernel in the > usual way, install kernel, reboot, install new world. Do not make > shortcuts in the update procedure. Hey Kostik, On my HardenedBSD system, world compiled fine with the patch, but the kernel didn't compile. I've uploaded the log to: https://hardenedbsd.org/~shawn/2017-05-15_ino64-r01.log This was from importing the patch from Phabricator into hardened/current/master in HardenedBSD. Thanks, -- Shawn Webb Cofounder and Security Engineer HardenedBSD GPG Key ID: 0x6A84658F52456EEE GPG Key Fingerprint: 2ABA B6BD EF6A F486 BE89 3D9E 6A84 658F 5245 6EEE signature.asc Description: PGP signature
64-bit inodes (ino64) Status Update and Call for Testing
Inodes are data structures corresponding to objects in a file system, such as files and directories. FreeBSD has historically used 32-bit values to identify inodes, which limits file systems to somewhat under 2^32 objects. Many modern file systems internally use 64-bit identifiers and FreeBSD needs to follow suit to properly and fully support these file systems. The 64-bit inode project, also known as ino64, started life many years ago as a project by Gleb Kurtsou (gleb@). After that time several people have had a hand in updating it and addressing regressions, after mckusick@ picked up and updated the patch, and acted as a flag-waver. Sponsored by the FreeBSD Foundation I have spent a significant effort on outstanding issues and integration -- fixing compat32 ABI, NFS and ZFS, addressing ABI compat issues and investigating and fixing ports failures. rmacklem@ provided feedback on NFS changes, emaste@ and jhb@ provided feedback and review on the ABI transition support. pho@ performed extensive testing and identified a number of issues that have now been fixed. kris@ performed an initial ports investigation followed by an exp-run by antoine@. emaste@ helped with organization of the process. This note explains how to perform useful testing of the ino64 branch, beyond typical smoke tests. 1. Overview. The ino64 branch extends the basic system types ino_t and dev_t from 32-bit to 64-bit, and nlink_t from 16-bit to 64-bit. The struct dirent layout is modified due to the larger size of ino_t, and also gains a d_off (directory offset) member. As ino64 implies an ABI change anyway the struct statfs f_mntfromname[] and f_mntonname[] array length MNAMELEN is increased from 88 to 1024, to allow for longer mount path names. ABI breakage is mitigated by providing compatibility using versioned symbols, ingenious use of the existing padding in structures, and by employing other tricks. Unfortunately, not everything can be fixed, especially outside the base system. For instance, third-party APIs which pass struct stat around are broken in backward and forward- incompatible way. 2. Motivation. The main risk of the ino64 change is the uncontrolled ABI breakage. Due to expansion of the basic types dev_t, ino_t and struct dirent, the impact is not limited to one part of the system, but affects: - kernel/userspace interface (syscalls ABI, mostly stat(2), kinfo and more) - libc interface (mostly related to the readdir(3), FTS(3)) - collateral damage in other libraries that happens to use changed types in the interfaces. See, for instance, libprocstat, for which compat was provided using symbol versioning, and libutil, which shlib version was bumped. 3. Quirks. We handled kinfo sysctl MIBs, but other MIBs which report structures depended on the changed type, are not handled in general. It was considered that the breakage is either in the management interfaces, where we usually allow ABI slip, or is not important. Struct xvnode changed layout, no compat shims are provided. For struct xtty, dev_t tty device member was reduced to uint32_t. It was decided that keeping ABI compat in this case is more useful than reporting 64bit dev_t, for the sake of pstat. 4. Testing procedure. The ino64 project can be tested by cloning the project branch from GitHub or by applying the patch to a working tree. The authorative source is the GitHub, I do not promise to update the review for each update. To clone from GitHub: % git clone -b ino64 https://github.com/FreeBSDFoundation/freebsd.git ino64 To fetch the patch from Phabricator: - Visit https://reviews.freebsd.org/D10439 - Click "Download Raw Diff" at the upper right of the page Or % arc patch D10439 After that, in the checkout directory do % (cd sys/kern && touch syscalls.master && make sysent) % (cd sys/compat/freebsd32 && touch syscalls.master && make sysent) If you use custom kernel configuration, ensure that options COMPAT_FREEBSD11 is included into the config. Then build world and kernel in the usual way, install kernel, reboot, install new world. Do not make shortcuts in the update procedure. 4.1 New kernel, old world. Build and install pristine HEAD world, apply patch and only build and install updated kernel. The system must work same as with the pristine kernel. 4.2 New kernel, new world, old third-party applications. Build and install patched kernel and world. Applications compiled on the pristine HEAD (e.g. installed by pkg from the regular portmgr builds) must work without a regression. 4.3 32bit compat. Same as 4.1 and 4.2, but for 32bit (i386) binaries on the amd64 host. Note that big-endian host, like powerpc, might expose additional bugs in the 32bit compat with the patch, but the testing is too cumbersome to arrange. 4.4 Targeted tests. Useful programs to check items 4.1, 4.2 and 4.3 are versions of the following programs, taken from the pristine system: stat(8). Use it on regular file, file in /dev, socket, pipe and
