Applied, thanks!
Milos Nikic, le ven. 27 mars 2026 20:54:58 -0700, a ecrit:
> In diskfs_lookup (and its variants), the enum lookup_type is passed in and
> expected. However, this base type is sometimes OR'd with SPEC_DOTDOT,
> creating a bitwise value (e.g., 268435459 on 64-bit architectures) that
> falls outside the valid range of the lookup_type enum. In modern C, this
> results in Undefined Behavior (UB) and triggers static analyzer warnings.
>
> Replace the use of the raw enum in these function signatures with a
> dedicated typedef (lookup_flags_t). This avoids the UB, modernizes the
> API, and safely signals that bitwise math is expected, improving overall
> type safety.
>
> This patch also adds include diskfs.h in places where its needed, and removes
> one priv.h from where it isn't needed.
> ---
> ext2fs/dir.c | 12 +++++++-----
> fatfs/dir.c | 12 +++++++-----
> isofs/lookup.c | 10 ++++++----
> libdiskfs/diskfs.h | 11 +++++++++--
> libdiskfs/lookup.c | 20 ++++++++++----------
> tmpfs/dir.c | 11 ++++++-----
> 6 files changed, 45 insertions(+), 31 deletions(-)
>
> diff --git a/ext2fs/dir.c b/ext2fs/dir.c
> index 932340a4..55f26579 100644
> --- a/ext2fs/dir.c
> +++ b/ext2fs/dir.c
> @@ -20,6 +20,7 @@
> Foundation, Inc., 675 Mass Ave, Cambridge, MA 02139, USA. */
>
> #include "ext2fs.h"
> +#include <libdiskfs/diskfs.h>
>
> #include <string.h>
> #include <stdio.h>
> @@ -135,7 +136,7 @@ static const unsigned char file_type_ext2[] =
> /* Implement the diskfs_lookup from the diskfs library. See
> <hurd/diskfs.h> for the interface specification. */
> error_t
> -diskfs_lookup_hard (struct node *dp, const char *name, enum lookup_type type,
> +diskfs_lookup_hard (struct node *dp, const char *name, lookup_flags_t
> l_flags,
> struct node **npp, struct dirstat *ds, struct protid *cred)
> {
> error_t err;
> @@ -145,22 +146,23 @@ diskfs_lookup_hard (struct node *dp, const char *name,
> enum lookup_type type,
> struct node *np = 0;
> ino_t retry_dotdot = 0;
> vm_prot_t prot =
> - (type == LOOKUP) ? VM_PROT_READ : (VM_PROT_READ | VM_PROT_WRITE);
> + (l_flags == LOOKUP) ? VM_PROT_READ : (VM_PROT_READ | VM_PROT_WRITE);
> memory_object_t memobj;
> vm_address_t buf = 0;
> vm_size_t buflen = 0;
> vm_address_t blockaddr;
> int idx, lastidx;
> int looped;
> + enum lookup_type type;
>
> - if ((type == REMOVE) || (type == RENAME))
> + if ((l_flags == REMOVE) || (l_flags == RENAME))
> assert_backtrace (npp);
>
> if (npp)
> *npp = 0;
>
> - spec_dotdot = type & SPEC_DOTDOT;
> - type &= ~SPEC_DOTDOT;
> + spec_dotdot = l_flags & SPEC_DOTDOT;
> + type = l_flags & ~SPEC_DOTDOT;
>
> namelen = strlen (name);
>
> diff --git a/fatfs/dir.c b/fatfs/dir.c
> index 1c71d68d..dbe243c6 100644
> --- a/fatfs/dir.c
> +++ b/fatfs/dir.c
> @@ -27,6 +27,7 @@
> #include <hurd/fsys.h>
>
> #include "fatfs.h"
> +#include <libdiskfs/diskfs.h>
>
> /* The size of a directory block is usually just the cluster size.
> However, the root directory of FAT12/16 file systems is stored in
> @@ -172,7 +173,7 @@ fatnamematch (const char *dirname, const char *username,
> size_t unamelen)
> /* Implement the diskfs_lookup callback from the diskfs library. See
> <hurd/diskfs.h> for the interface specification. */
> error_t
> -diskfs_lookup_hard (struct node *dp, const char *name, enum lookup_type type,
> +diskfs_lookup_hard (struct node *dp, const char *name, lookup_flags_t
> l_flags,
> struct node **npp, struct dirstat *ds, struct protid *cred)
> {
> error_t err;
> @@ -182,22 +183,23 @@ diskfs_lookup_hard (struct node *dp, const char *name,
> enum lookup_type type,
> struct node *np = 0;
> int retry_dotdot = 0;
> vm_prot_t prot =
> - (type == LOOKUP) ? VM_PROT_READ : (VM_PROT_READ | VM_PROT_WRITE);
> + (l_flags == LOOKUP) ? VM_PROT_READ : (VM_PROT_READ | VM_PROT_WRITE);
> memory_object_t memobj;
> vm_address_t buf = 0;
> vm_size_t buflen = 0;
> int blockaddr;
> int idx, lastidx;
> int looped;
> + enum lookup_type type;
>
> - if ((type == REMOVE) || (type == RENAME))
> + if ((l_flags == REMOVE) || (l_flags == RENAME))
> assert_backtrace (npp);
>
> if (npp)
> *npp = 0;
>
> - spec_dotdot = type & SPEC_DOTDOT;
> - type &= ~SPEC_DOTDOT;
> + spec_dotdot = l_flags & SPEC_DOTDOT;
> + type = l_flags & ~SPEC_DOTDOT;
>
> namelen = strlen (name);
>
> diff --git a/isofs/lookup.c b/isofs/lookup.c
> index b55aef96..d9322fac 100644
> --- a/isofs/lookup.c
> +++ b/isofs/lookup.c
> @@ -21,6 +21,7 @@
> #include <string.h>
> #include <stdlib.h>
> #include <dirent.h>
> +#include <libdiskfs/diskfs.h>
> #include "isofs.h"
>
> /* From inode.c */
> @@ -66,7 +67,7 @@ isonamematch (const char *dirname, size_t dnamelen,
> /* Implement the diskfs_lookup callback from the diskfs library. See
> <hurd/diskfs.h> for the interface specification. */
> error_t
> -diskfs_lookup_hard (struct node *dp, const char *name, enum lookup_type type,
> +diskfs_lookup_hard (struct node *dp, const char *name, lookup_flags_t
> l_flags,
> struct node **npp, struct dirstat *ds, struct protid *cred)
> {
> error_t err = ENOENT;
> @@ -76,15 +77,16 @@ diskfs_lookup_hard (struct node *dp, const char *name,
> enum lookup_type type,
> void *buf;
> void *blockaddr;
> ino_t id;
> + enum lookup_type type;
>
> - if ((type == REMOVE) || (type == RENAME))
> + if ((l_flags == REMOVE) || (l_flags == RENAME))
> assert_backtrace (npp);
>
> if (npp)
> *npp = 0;
>
> - spec_dotdot = type & SPEC_DOTDOT;
> - type &= ~SPEC_DOTDOT;
> + spec_dotdot = l_flags & SPEC_DOTDOT;
> + type = l_flags & ~SPEC_DOTDOT;
>
> namelen = strlen (name);
>
> diff --git a/libdiskfs/diskfs.h b/libdiskfs/diskfs.h
> index 5f832dd7..8f5db533 100644
> --- a/libdiskfs/diskfs.h
> +++ b/libdiskfs/diskfs.h
> @@ -146,6 +146,13 @@ enum lookup_type
> RENAME,
> };
>
> +/**
> + * Represents a lookup_type optionally OR'd with lookup flags (SPEC_DOTDOT)
> + * We use a typedef rather than the raw enum to safely allow bitwise math
> + * without triggering undefined behavior or static analyzer warnings.
> + */
> +typedef int lookup_flags_t;
> +
> /* Pending directory and file modification request */
> struct modreq
> {
> @@ -352,7 +359,7 @@ error_t diskfs_set_statfs (fsys_statfsbuf_t *statfsbuf);
> Return EIO if appropriate.
> */
> error_t diskfs_lookup_hard (struct node *dp,
> - const char *name, enum lookup_type type,
> + const char *name, lookup_flags_t l_flags,
> struct node **np, struct dirstat *ds,
> struct protid *cred);
>
> @@ -815,7 +822,7 @@ diskfs_disknode_node (struct disknode *disknode)
> This function is a wrapper for diskfs_lookup_hard.
> */
> error_t diskfs_lookup (struct node *dp,
> - char *name, enum lookup_type type,
> + char *name, lookup_flags_t l_flags,
> struct node **np, struct dirstat *ds,
> struct protid *cred);
>
> diff --git a/libdiskfs/lookup.c b/libdiskfs/lookup.c
> index 201613c3..87b54c01 100644
> --- a/libdiskfs/lookup.c
> +++ b/libdiskfs/lookup.c
> @@ -18,7 +18,7 @@
> along with this program; if not, write to the Free Software
> Foundation, Inc., 59 Temple Place - Suite 330, Boston, MA 02111, USA. */
>
> -#include "priv.h"
> +#include <libdiskfs/diskfs.h>
> #include <string.h>
>
> /* Lookup in directory DP (which is locked) the name NAME. TYPE will
> @@ -71,13 +71,13 @@
>
> This function is a wrapper for diskfs_lookup_hard. */
> error_t
> -diskfs_lookup (struct node *dp, char *name, enum lookup_type type,
> +diskfs_lookup (struct node *dp, char *name, lookup_flags_t l_flags,
> struct node **np, struct dirstat *ds, struct protid *cred)
> {
> error_t err;
> struct node *cached;
>
> - if (type == REMOVE || type == RENAME)
> + if (l_flags == REMOVE || l_flags == RENAME)
> assert_backtrace (np);
>
> if (!S_ISDIR (dp->dn_stat.st_mode))
> @@ -133,7 +133,7 @@ diskfs_lookup (struct node *dp, char *name, enum
> lookup_type type,
> return EAGAIN;
> }
>
> - if (type == LOOKUP)
> + if (l_flags == LOOKUP)
> /* Check the cache first */
> cached = diskfs_check_lookup_cache (dp, name);
> else
> @@ -163,13 +163,13 @@ diskfs_lookup (struct node *dp, char *name, enum
> lookup_type type,
> }
> else
> {
> - err = diskfs_lookup_hard (dp, name, type, np, ds, cred);
> + err = diskfs_lookup_hard (dp, name, l_flags , np, ds, cred);
> if (err && err != ENOENT)
> return err;
>
> - if (type == RENAME
> - || (type == CREATE && err == ENOENT)
> - || (type == REMOVE && err != ENOENT))
> + if (l_flags == RENAME
> + || (l_flags == CREATE && err == ENOENT)
> + || (l_flags == REMOVE && err != ENOENT))
> {
> error_t err2;
>
> @@ -193,9 +193,9 @@ diskfs_lookup (struct node *dp, char *name, enum
> lookup_type type,
> }
> }
>
> - if ((type == LOOKUP || type == CREATE) && !err && np)
> + if ((l_flags == LOOKUP || l_flags == CREATE) && !err && np)
> diskfs_enter_lookup_cache (dp, *np, name);
> - else if (type == LOOKUP && err == ENOENT)
> + else if (l_flags == LOOKUP && err == ENOENT)
> diskfs_enter_lookup_cache (dp, 0, name);
> }
>
> diff --git a/tmpfs/dir.c b/tmpfs/dir.c
> index 8af2d070..75242b52 100644
> --- a/tmpfs/dir.c
> +++ b/tmpfs/dir.c
> @@ -21,6 +21,7 @@
> #include <unistd.h>
> #include <sys/mman.h>
>
> +#include <libdiskfs/diskfs.h>
> #include "tmpfs.h"
> #include <stdlib.h>
>
> @@ -169,18 +170,18 @@ diskfs_drop_dirstat (struct node *dp, struct dirstat
> *ds)
>
> error_t
> diskfs_lookup_hard (struct node *dp,
> - const char *name, enum lookup_type type,
> + const char *name, lookup_flags_t l_flags,
> struct node **np, struct dirstat *ds,
> struct protid *cred)
> {
> const size_t namelen = strlen (name);
> struct tmpfs_dirent *d, **prevp;
>
> - if (type == REMOVE || type == RENAME)
> + if (l_flags == REMOVE || l_flags == RENAME)
> assert_backtrace (np);
>
> if (ds)
> - ds->dotdot = type & SPEC_DOTDOT;
> + ds->dotdot = l_flags & SPEC_DOTDOT;
>
> if (namelen == 1 && name[0] == '.')
> {
> @@ -200,7 +201,7 @@ diskfs_lookup_hard (struct node *dp,
> if (dddn == 0) /* root directory */
> return EAGAIN;
>
> - if (type == (REMOVE|SPEC_DOTDOT) || type == (RENAME|SPEC_DOTDOT))
> + if (l_flags == (REMOVE|SPEC_DOTDOT) || l_flags == (RENAME|SPEC_DOTDOT))
> {
> *np = *dddn->hprevp;
> assert_backtrace (*np);
> @@ -213,7 +214,7 @@ diskfs_lookup_hard (struct node *dp,
> pthread_mutex_unlock (&dp->lock);
> err = diskfs_cached_lookup ((ino_t) (intptr_t) dddn, np);
>
> - if (type == (LOOKUP|SPEC_DOTDOT))
> + if (l_flags == (LOOKUP|SPEC_DOTDOT))
> diskfs_nrele (dp);
> else
> pthread_mutex_lock (&dp->lock);
> --
> 2.53.0
>
>
--
Samuel
<A> mr - remove the home of correct users who accidentally enter mr
<A> instead of rm
