On Thu, Jun 12, 2025 at 02:13:04PM -0230, Theodore Ts'o wrote: > On Wed, Jun 11, 2025 at 09:44:17AM -0700, Darrick J. Wong wrote: > > From: Darrick J. Wong <djwong@xxxxxxxxxx> > > > > 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. I.... had no idea that errcode_t's were actually segmented numbers. Is there a way to figure out the subsystem from one of them? Or will fuse2fs just have to "know" that !(errcode & 0xFFFFFF00) means "errno"? > 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. Hrm -- if MMP fails, that implies that we might not be the owner of this filesystem, right? Doesn't that means we should be careful about not scribbling on the superblock? > commit 71f046a788adbae163c9398fccf50fff89bb9083 > Author: Theodore Ts'o <tytso@xxxxxxx> > 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@xxxxxxxxxx> > Signed-off-by: Theodore Ts'o <tytso@xxxxxxx> > > 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; Ok I'll rework the patch with this logic, but add a fat comment about all this. --D > break; > } > > -no_translation: > if (!is_err) > return ret; > >
