dir: Introduce struct DirEntryFlex

The directory package as implemented in AFS-2 allocates space for each
directory entry as a DirEntry struct followed by 0-8 contiguous
DirXEntry structs, as needed. This is implemented by:

 - afs_dir_NameBlobs    calculates the number of blocks needed
 - FindBlobs		allocates and returns index of entry
 - afs_dir_GetBlob	returns pointer to 1st DirEntry struct

After this, we populate DirEntry (and any contiguous DirXEntry blocks)
with open code.  Most existing code writes the entry's name via a string
copy operation to DirEntry->name, which is only 16 bytes long.
Therefore, for dir entry names that are 16 bytes or longer, OpenAFS
routinely does string copies that look like buffer overruns.  This has
not previously caused problems because the OpenAFS code has arranged for
a sufficiently large amount of contiguous memory to be available.
However, this remains undefined behavior in the C abstract virtual
machine; thus compilers are not required to produce safe operation.

Recent changes in the OpenAFS build chain have made this approach no
longer viable:

1) Linux 6.5 commit df8fc4e934c12b 'kbuild: Enable
-fstrict-flex-arrays=3' modified the hardening of several kernel
string operations when running with CONFIG_FORTIFY_SOURCE=y.

2) gcc 13 commit 79a89108dd352cd9288f5de35481b1280c7588a5
'__builtin_dynamic_object_size: Recognize builtin' provides some
enhancements to _builtin_object_size.  The Linux commit above will now
use these when the kernel is built with gcc 13.

When OpenAFS is built under Linux 6.5 or higher and gcc 13 or higher,
the hardened strlcpy will BUG for directory entry names longer than 16
characters.

Since there are multiple places where OpenAFS writes directory names,
there are several symptoms that may manifest.  However, the first one is
usually a kernel BUG at cache manager initialization if running with
afsd -dynroot _and_ there are any cell names 15 characters or longer in
the client CellServDB.  (A 15-character cellname reaches the 16
character limit when -dyrnoot adds the RW mountpoint ".<cellname>".)

Address this by using flexible arrays (standardized with C99). A
flexible array is a variable-length array that is declared with no size
at all, e.g., name[].

Create an autoconf test to determine whether the compiler supports
flexible arrays.

Create a new struct DirEntryFlex.  If the compiler supports
flexible arrays, define name[]; otherwise retain the name[16]
definition.

Whenever we write a directory name, use DirEntryFlex so that any
hardening will be satisfied that there is sufficient space for the name.

However, the actual guarantee that this is true is still provided by the
OpenAFS directory routines mentioned above - all of these remain
unchanged.

The DirEntry struct remains unchanged for continued use in OpenAFS, as
well as for any out-of-tree users of the directory package.

Reviewed-on: https://gerrit.openafs.org/15573
Tested-by: BuildBot <buildbot@rampaginggeek.com>
Reviewed-by: Cheyenne Wills <cwills@sinenomine.net>
Reviewed-by: Benjamin Kaduk <kaduk@mit.edu>
Reviewed-by: Michael Meffie <mmeffie@sinenomine.net>
(cherry picked from commit e2ec16cf94)

Change-Id: Ibf6d3549ba1e941c957e98ef4875152d865c9358
Reviewed-on: https://gerrit.openafs.org/15599
Reviewed-by: Michael Meffie <mmeffie@sinenomine.net>
Reviewed-by: Andrew Deason <adeason@sinenomine.net>
Reviewed-by: Mark Vitale <mvitale@sinenomine.net>
Reviewed-by: Michael Laß <lass@mail.uni-paderborn.de>
Tested-by: BuildBot <buildbot@rampaginggeek.com>
Reviewed-by: Benjamin Kaduk <kaduk@mit.edu>
This commit is contained in:
Mark Vitale 2023-09-15 15:01:56 -04:00 committed by Benjamin Kaduk
parent 9ff2623126
commit fd527549c2
6 changed files with 49 additions and 6 deletions

View File

@ -56,6 +56,7 @@ OPENAFS_HCRYPTO
OPENAFS_CURSES
OPENAFS_C_ATTRIBUTE
OPENAFS_C_PRAGMA
OPENAFS_C_FLEXIBLE_ARRAY
OPENAFS_MORE_ROKEN_CHECKS
OPENAFS_NETDB_CHECKS
OPENAFS_ROKEN_HEADERS

View File

@ -426,7 +426,7 @@ afs_linux_readdir(struct file *fp, void *dirbuf, filldir_t filldir)
int code;
int offset;
afs_int32 dirpos;
struct DirEntry *de;
struct DirEntryFlex *de;
struct DirBuffer entry;
ino_t ino;
int len;
@ -531,7 +531,7 @@ afs_linux_readdir(struct file *fp, void *dirbuf, filldir_t filldir)
goto unlock_out;
}
de = (struct DirEntry *)entry.data;
de = entry.data;
ino = afs_calc_inum (avc->f.fid.Cell, avc->f.fid.Fid.Volume,
ntohl(de->fid.vnode));
len = strlen(de->name);

View File

@ -228,7 +228,7 @@ afs_dynroot_addDirEnt(struct DirHeader *dirHeader, int *curPageP,
{
char *dirBase = (char *)dirHeader;
struct PageHeader *pageHeader;
struct DirEntry *dirEntry;
struct DirEntryFlex *dirEntry;
int sizeOfEntry, i, t1, t2;
int curPage = *curPageP;
int curChunk = *curChunkP;
@ -257,7 +257,7 @@ afs_dynroot_addDirEnt(struct DirHeader *dirHeader, int *curPageP,
dirHeader->alloMap[curPage] = EPP - 1;
}
dirEntry = (struct DirEntry *)(pageHeader + curChunk);
dirEntry = (struct DirEntryFlex *)(pageHeader + curChunk);
dirEntry->flag = 1;
dirEntry->length = 0;
dirEntry->next = 0;

View File

@ -0,0 +1,16 @@
AC_DEFUN([OPENAFS_C_FLEXIBLE_ARRAY],[
dnl Check to see if the compiler support C99 flexible arrays, e.g., var[]
AC_MSG_CHECKING([for C99 flexible arrays])
AC_COMPILE_IFELSE([AC_LANG_PROGRAM([[
struct flexarraytest {
int flag;
int numbers[];
};
]], [[]])
],
[AC_MSG_RESULT([yes])
AC_DEFINE([HAVE_FLEXIBLE_ARRAY], [1],
[Define to 1 if your compiler supports C99 flexible arrays.])
],[AC_MSG_RESULT([no])]
)
])

View File

@ -97,7 +97,7 @@ afs_dir_Create(dir_file_t dir, char *entry, void *voidfid)
int blobs, firstelt;
int i;
struct DirBuffer entrybuf, prevbuf, headerbuf;
struct DirEntry *ep;
struct DirEntryFlex *ep;
struct DirHeader *dhp;
int code;
size_t rlen;
@ -125,7 +125,7 @@ afs_dir_Create(dir_file_t dir, char *entry, void *voidfid)
/* First, we fill in the directory entry. */
if (afs_dir_GetBlob(dir, firstelt, &entrybuf) != 0)
return EIO;
ep = (struct DirEntry *)entrybuf.data;
ep = entrybuf.data;
ep->flag = FFIRST;
ep->fid.vnode = htonl(vfid[1]);

View File

@ -51,6 +51,32 @@ struct DirHeader {
unsigned short hashTable[NHASHENT];
};
/*
* This struct is just a copy of DirEntry, but with name defined as a flexible
* array if possible.
*
* Using this helps us convince safety-minded string functions (e.g.
* _FORTIFY_SOURCE) that an OpenAFS directory entry name really does fit
* in the allotted space, and thus avoid undefined behavior.
*/
struct DirEntryFlex {
char flag;
char length; /* currently unused */
unsigned short next;
struct MKFid fid;
#ifdef HAVE_FLEXIBLE_ARRAY
char name[];
#else
char name[16];
#endif
};
/*
* This struct was the original format for directory entries in very early
* versions of AFS. But now it just represents the minimum possible on-disk
* representation of a directory entry. The 16-character limit was relieved by
* the introduction of extension struct DirXEntry in AFS-2.
*/
struct DirEntry {
/* A directory entry */
char flag;