Re: [PATCH 03/17] mount_service: create high level fuse helpers

["Darrick J. Wong" <djwong@kernel.org>]

On Mon, Mar 30, 2026 at 08:51:01PM +0000, Bernd Schubert wrote:
> On 3/30/26 22:30, Darrick J. Wong wrote:
> > On Mon, Mar 30, 2026 at 09:37:18PM +0200, Bernd Schubert wrote:
> >> Hi Darrick,
> >>
> >> On 3/27/26 02:25, Darrick J. Wong wrote:
> >>> From: Darrick J. Wong <djwong@kernel.org>
> >>>
> >>> Create a fuse_main wrapper for fuse services.
> >>>
> >>> Signed-off-by: Darrick J. Wong <djwong@kernel.org>
> >>> ---
> >>>  include/fuse.h         |   33 ++++++++++++++++++++++++++++
> >>>  lib/fuse_versionscript |    1 +
> >>>  lib/helper.c           |   57 +++++++++++++++++++++++++++++++++++++++++-------
> >>>  3 files changed, 83 insertions(+), 8 deletions(-)
> >>>
> >>>
> >>> diff --git a/include/fuse.h b/include/fuse.h
> >>> index 595cac07f2be36..c7dae040857aca 100644
> >>> --- a/include/fuse.h
> >>> +++ b/include/fuse.h
> >>> @@ -1008,6 +1008,39 @@ static inline int fuse_main_fn(int argc, char *argv[],
> >>>  #define fuse_main(argc, argv, op, user_data) \
> >>>  	fuse_main_fn(argc, argv, op, user_data)
> >>>  
> >>> +#if FUSE_MAKE_VERSION(3, 19) <= FUSE_USE_VERSION
> >>> +struct fuse_service;
> >>> +int fuse_service_main_real_versioned(struct fuse_service *service,
> >>> +				     int argc, char *argv[],
> >>> +				     const struct fuse_operations *op,
> >>> +				     size_t op_size,
> >>> +				     struct libfuse_version *version,
> >>> +				     void *user_data);
> >>> +
> >>> +/**
> >>> + * Same as fuse_service_main_fn, but takes its information from the mount
> >>> + * service context.
> >>> + */
> >>> +static inline int fuse_service_main_fn(struct fuse_service *service,
> >>> +				       int argc, char *argv[],
> >>> +				       const struct fuse_operations *op,
> >>> +				       void *user_data)
> >>> +{
> >>> +	struct libfuse_version version = {
> >>> +		.major  = FUSE_MAJOR_VERSION,
> >>> +		.minor  = FUSE_MINOR_VERSION,
> >>> +		.hotfix = FUSE_HOTFIX_VERSION,
> >>> +		.padding = 0
> >>> +	};
> >>> +
> >>> +	return fuse_service_main_real_versioned(service, argc, argv, op,
> >>> +						sizeof(*(op)), &version,
> >>> +						user_data);
> >>> +}
> >>> +#define fuse_service_main(s, argc, argv, op, user_data) \
> >>> +	fuse_service_main_fn(s, argc, argv, op, user_data)
> >>> +#endif /* FUSE_USE_VERSION >= FUSE_MAKE_VERSION(3, 19) */
> >>
> >> sorry, this is not exactly what I meant. Could you take a look here? I
> > 
> > OH, you want the program to pass in both the version of libfuse that the
> > program was built against (struct libfuse_version) and the version of
> > libfuse that the program asked for (FUSE_USE_VERSION).  Not just header
> > guards.
> 
> I don't think we need the header guards, I think these are more for the
> case the API changes. With just additions there cannot break anything?
> 
> > 
> >> just pushed the unfinished patch I made in December. It got complicated
> >> in the high level API and I didn't have time to finish it yet.
> > 
> > Yes, that does make sense for setting defaults based on the version of
> > the libfuse API that the fuse server expects.
> > 
> >> https://github.com/libfuse/libfuse/pull/1382/changes/24836ded18e9736eb4691600600ff1d7cf581e29
> >>
> >> Basically I would like to add the API version to the functions, in order
> >> to set defaults flags. The topic came up about FUSE_CAP_AUTO_INVAL_DATA,
> >> because it can cause data corruption, but there is also a risk to cause
> >> regressions. So would be good to set that based on the API version
> >> (assuming people read the rather new doc/ChangeLog-API.rst).
> >>
> >> Anyway your function should basically be
> >>
> >> int fuse_service_main_real_versioned(struct fuse_service *service,
> >> 				     int argc, char *argv[],
> >> 				     const struct fuse_operations *op,
> >> 				     size_t op_size,
> >>                                      unsigned int user_apiabi_version,
> >> 				     struct libfuse_version *version,
> >> 				     void *user_data);
> >>
> >>
> >> I.e. adding in 'user_apiabi_version'. Although while looking into this
> >> again, maybe we should mis-use 'padding' and rename it 'api version. It
> >> would definitely simplify things.
> >>
> >>
> >> While you look into all these things, what is your opinion? ;)
> > 
> > I like the idea of encoding the FUSE_USE_VERSION as the padding value.
> > I wonder if there's a danger of a client setting the fields manually:
> > 
> > 	struct libfuse_version version;
> > 
> > 	version.major = FUSE_MAJOR_VERSION;
> > 	version.minor = FUSE_MINOR_VERSION;
> > 	version.hotfix = FUSE_HOTFIX_VERSION;
> > 
> > 	fuse_main_real_versioned(..., &version...);
> > 
> > such that version.padding is now set to stack garbage?  The wrapper
> > functions in fuse.h use struct initialization so the .padding value will
> > be zero, but fuse_session_new_versioned doesn't appear to require zero,
> > which makes such a change a bit risky.
> > 
> > Debian codesearch seems to think there aren't any direct callers of
> > fuse_session_new_versioned or fuse_main_real_versioned outside of fuse,
> > so this might not be a big risk.
> > 
> > If the risk is acceptable, it would reduce the amount of code changes to
> > add the FUSE_USE_VERSION number and force programs to #define it before
> > #include'ing fuse.h.
> > 
> > The existing functions that take a struct libfuse_version could just
> > ignore values like 0 or values that arent't in the range 30-39 or
> > 310-319.  That might be enough to avoid complaints.
> 
> Hmm yeah, there a bit risk people people who access these functions with
> dlopen. My personal opinion is that one should not directly access
> libfuse functions, but write a wrapper lib, correctly define the API
> version in that lib and then dlopen the wrapper lib.
> I need to think about it again, thanks for pointing that out!

libfuse could restrict itself to failing only 1-29 and 40-309, and
warning on otherwise crazy versions:

#define FUSE_MAKE_VERSION_OLD(maj, min)  ((maj) * 10 + (min))

static inline bool validate_version(const struct libfuse_version *version,
				    size_t op_size)
{
	if (version == NULL) {
		fuse_log(FUSE_LOG_ERR, "fuse: warning: version not passed to fuse_session_new()\n");
		return false;
	}

	/* padding got turned into fuse api level, so zero is still accepted */
	if (version->padding == 0)
		return true;

	/* 3.0 -> 3.9 had one format for FUSE_MAKE_VERSION */
	if (version->padding >= FUSE_MAKE_VERSION_OLD(3, 0) &&
	    version->padding <= FUSE_MAKE_VERSION_OLD(3, 9))
		return true;

	/* 3.10+ has a different format for FUSE_MAKE_VERSION */
	if (version->padding >= FUSE_MAKE_VERSION(FUSE_MAJOR_VERSION, 0)) {
		/*
		 * Warn about too-new versions if we aren't going to print the
		 * op_size warning below.
		 */
		if (version->padding > FUSE_VERSION &&
		    sizeof(struct fuse_lowlevel_ops) >= op_size)
			fuse_log(FUSE_LOG_ERR,
				 "fuse: warning: library too old, some operations may not work\n");

		return true;
	}

	fuse_log(FUSE_LOG_ERR, "fuse: warning: version %u not recognized\n",
		 version->padding);
	return false;
}

A good question is, what format will FUSE_MAKE_VERSION have with 4.x?. :)

--D

> 
> Cheers,
> Bernd