Message-ID: <20250612164304.GQ784455@mit.edu>
Date: Thu, 12 Jun 2025 14:13:04 -0230
From: "Theodore Ts'o" <tytso@....edu>
To: "Darrick J. Wong" <djwong@...nel.org>
Cc: linux-ext4@...r.kernel.org
Subject: Re: [PATCH 3/3] fuse2fs: catch positive errnos coming from libext2fs

On Wed, Jun 11, 2025 at 09:44:17AM -0700, Darrick J. Wong wrote:
> From: Darrick J. Wong <djwong@...nel.org>
> 
> Something in libext2fs is returning EOVERFLOW (positive) to us.  We need
> to pass negative errnos back to the fuse driver, so perform this
> translation.

This isn't actually the best way to fix things.  The way the com_err
architecture works is that errcode_t is a 32-bit unsigned integer,
where the the top 24-bits is a subsystem identifier.  If the subsystem
identifier is zero, then the low 8 bits is presumed to be an errno
value.  Otherwise, the subsystem identifier is formed by taking a 4
character identifier from 62 valid code points A-Z, a-z, 0-9, and _,
where A is 1, and _ is 63.

Error table subsystems that are in common use and used by packages
shipped by most Linux distributions include "krb5" and "kadm" from the
MIT Kerberos implementation, and "ext2" and "prof" which ship as part
of e2fsprogs.  The original design came from Multics, and the idea is
that a library might call other libraries, and having a single unified
error code namespace can be super-useful.  Top-level callers can check
to see if an error is non-zero, and if so, call error_message(retval)
and get back a human-friendly string regardless of which library might
have originally generated the error.

CMU has a handy-dandy registry of the various libraries that use the
common error infrastructure here[1].

[1] https://www.central.org/frameless/numbers/errors.html

In the case of the ext2fs library, it doesn't actually call any AFS,
Kerberos, ASN.1, etc. libraries, so in practice the only valid error
codes that we should get back are either in the range 0..255 and
EXT2_ET_BASE..EXT2_ET_BASE+255.  But at least in theory, it's possible
that in the future, libext2fs might call some other library that might
return com_err error codes.

So a better, more idiomatic fix would be something like this below.

       	       		      	      	- Ted

P.S.  By the way, I'm not entirely convinced by the is_err vs !is_err
logic.  I get the idea is that we want to not log certain error cases
resulting from looking up a non-existing file name, but for example,
EXT2_ET_NO_MEMORY or any of the EXT2_TDB_* error messages should never
happen under normal circumstances, so if they do happen, they probably
should be logged, and so perhaps is_err=1 should be set for those
errors.  Similarly, I suspect that any MMP errors should probably also
be logged, but we can handle that as a separate commit.


commit 71f046a788adbae163c9398fccf50fff89bb9083
Author: Theodore Ts'o <tytso@....edu>
Date:   Thu Jun 12 14:03:44 2025 -0230

    fuse2fs: correctly handle system errno values in __translate_error()
    
    Fixes: 81cbf1ef4f5dab ("misc: add fuse2fs, a FUSE server for e2fsprogs")
    Reported-by: "Darrick J. Wong" <djwong@...nel.org>
    Signed-off-by: Theodore Ts'o <tytso@....edu>

diff --git a/misc/fuse2fs.c b/misc/fuse2fs.c
index bc49edfe..97b1c5b5 100644
--- a/misc/fuse2fs.c
+++ b/misc/fuse2fs.c
@@ -4659,9 +4659,9 @@ static int __translate_error(ext2_filsys fs, ext2_ino_t ino, errcode_t err,
 	int is_err = 0;
 
 	/* Translate ext2 error to unix error code */
-	if (err < EXT2_ET_BASE)
-		goto no_translation;
 	switch (err) {
+	case 0:
+		break;
 	case EXT2_ET_NO_MEMORY:
 	case EXT2_ET_TDB_ERR_OOM:
 		ret = -ENOMEM;
@@ -4755,11 +4755,10 @@ static int __translate_error(ext2_filsys fs, ext2_ino_t ino, errcode_t err,
 		break;
 	default:
 		is_err = 1;
-		ret = -EIO;
+		ret = (err < 256) ? -err : -EIO;
 		break;
 	}
 
-no_translation:
 	if (!is_err)
 		return ret;
 

Powered by blists - more mailing lists