[PATCH 0/2] fsconfig: minor fsparam_fd fixes

[PATCH 0/2] fsconfig: minor fsparam_fd fixes

[Aleksa Sarai]

While working on adding an fsparam_fd() argument to cgroupfs, I noticed
that there are only two users of fsparam_fd() and they both seemed to
have minor issues:

* autofs has a missing fput() when using FSCONFIG_SET_FD.
* coda uses fsparam_fd() but only supports string-based fds but
  FSCONFIG_SET_FD is more ergonomic when using the new mount API.

I have tested this logic for the cgroupfs patch but I have only compile
tested the fixes for autofs and coda.

Signed-off-by: Aleksa Sarai <cyphar@cyphar.com>
---
Aleksa Sarai (2):
      autofs: fix missing fput for FSCONFIG_SET_FD
      coda: support FSCONFIG_SET_FD for fd mount flag

 fs/autofs/inode.c |  3 +--
 fs/coda/inode.c   | 18 ++++++++++++------
 2 files changed, 13 insertions(+), 8 deletions(-)
---
base-commit: b80cc4df1124702c600fd43b784e423a30919204
change-id: 20240714-fsconfig-fsparam_fd-fixes-09f2e741c28d

Best regards,
-- 
Aleksa Sarai <cyphar@cyphar.com>

[PATCH 1/2] autofs: fix missing fput for FSCONFIG_SET_FD

[Aleksa Sarai]

If you pass an fd using FSCONFIG_SET_FD, autofs_parse_fd() "steals" the
param->file and so the fs_context infrastructure will not do fput() for
us.

Fixes: e6ec453bd0f0 ("autofs: convert autofs to use the new mount api")
Signed-off-by: Aleksa Sarai <cyphar@cyphar.com>
---
 fs/autofs/inode.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/fs/autofs/inode.c b/fs/autofs/inode.c
index 1f5db6863663..bb404bfce036 100644
--- a/fs/autofs/inode.c
+++ b/fs/autofs/inode.c
@@ -172,8 +172,7 @@ static int autofs_parse_fd(struct fs_context *fc, struct autofs_sb_info *sbi,
 	ret = autofs_check_pipe(pipe);
 	if (ret < 0) {
 		errorf(fc, "Invalid/unusable pipe");
-		if (param->type != fs_value_is_file)
-			fput(pipe);
+		fput(pipe);
 		return -EBADF;
 	}
 

-- 
2.45.2

[PATCH 2/2] coda: support FSCONFIG_SET_FD for fd mount flag

[Aleksa Sarai]

It's possible for users to pass file descriptors directly using
FSCONFIG_SET_FD, but the old version only supported the string-based
passing of file descriptors.

Fixes: 5916f439f2eb ("Convert coda to use the new mount API")
Signed-off-by: Aleksa Sarai <cyphar@cyphar.com>
---
 fs/coda/inode.c | 18 ++++++++++++------
 1 file changed, 12 insertions(+), 6 deletions(-)

diff --git a/fs/coda/inode.c b/fs/coda/inode.c
index 6898dc621011..df477a7218a2 100644
--- a/fs/coda/inode.c
+++ b/fs/coda/inode.c
@@ -122,21 +122,27 @@ static const struct fs_parameter_spec coda_param_specs[] = {
 static int coda_parse_fd(struct fs_context *fc, int fd)
 {
 	struct coda_fs_context *ctx = fc->fs_private;
-	struct fd f;
+	struct file *file;
 	struct inode *inode;
 	int idx;
 
-	f = fdget(fd);
-	if (!f.file)
+	if (param->type == fs_value_is_file) {
+		file = param->file;
+		param->file = NULL;
+	} else {
+		file = fget(result->uint_32);
+	}
+	if (!file)
 		return -EBADF;
-	inode = file_inode(f.file);
+
+	inode = file_inode(file);
 	if (!S_ISCHR(inode->i_mode) || imajor(inode) != CODA_PSDEV_MAJOR) {
-		fdput(f);
+		fput(file);
 		return invalf(fc, "code: Not coda psdev");
 	}
 
 	idx = iminor(inode);
-	fdput(f);
+	fput(file);
 
 	if (idx < 0 || idx >= MAX_CODADEVS)
 		return invalf(fc, "coda: Bad minor number");

-- 
2.45.2

Re: [PATCH 2/2] coda: support FSCONFIG_SET_FD for fd mount flag

[kernel test robot]

Hi Aleksa,

kernel test robot noticed the following build errors:

[auto build test ERROR on b80cc4df1124702c600fd43b784e423a30919204]

url:    https://github.com/intel-lab-lkp/linux/commits/Aleksa-Sarai/autofs-fix-missing-fput-for-FSCONFIG_SET_FD/20240718-230056
base:   b80cc4df1124702c600fd43b784e423a30919204
patch link:    https://lore.kernel.org/r/20240719-fsconfig-fsparam_fd-fixes-v1-2-7ccd315c2ad4%40cyphar.com
patch subject: [PATCH 2/2] coda: support FSCONFIG_SET_FD for fd mount flag
config: arm-randconfig-003-20240719 (https://download.01.org/0day-ci/archive/20240719/202407190741.8fA9KJbt-lkp@intel.com/config)
compiler: arm-linux-gnueabi-gcc (GCC) 14.1.0
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20240719/202407190741.8fA9KJbt-lkp@intel.com/reproduce)

If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202407190741.8fA9KJbt-lkp@intel.com/

All errors (new ones prefixed by >>):

   fs/coda/inode.c: In function 'coda_parse_fd':
>> fs/coda/inode.c:129:13: error: 'param' undeclared (first use in this function); did you mean 'parameq'?
     129 |         if (param->type == fs_value_is_file) {
         |             ^~~~~
         |             parameq
   fs/coda/inode.c:129:13: note: each undeclared identifier is reported only once for each function it appears in
>> fs/coda/inode.c:133:29: error: 'result' undeclared (first use in this function); did you mean 'mf_result'?
     133 |                 file = fget(result->uint_32);
         |                             ^~~~~~
         |                             mf_result


vim +129 fs/coda/inode.c

   121	
   122	static int coda_parse_fd(struct fs_context *fc, int fd)
   123	{
   124		struct coda_fs_context *ctx = fc->fs_private;
   125		struct file *file;
   126		struct inode *inode;
   127		int idx;
   128	
 > 129		if (param->type == fs_value_is_file) {
   130			file = param->file;
   131			param->file = NULL;
   132		} else {
 > 133			file = fget(result->uint_32);
   134		}
   135		if (!file)
   136			return -EBADF;
   137	
   138		inode = file_inode(file);
   139		if (!S_ISCHR(inode->i_mode) || imajor(inode) != CODA_PSDEV_MAJOR) {
   140			fput(file);
   141			return invalf(fc, "code: Not coda psdev");
   142		}
   143	
   144		idx = iminor(inode);
   145		fput(file);
   146	
   147		if (idx < 0 || idx >= MAX_CODADEVS)
   148			return invalf(fc, "coda: Bad minor number");
   149		ctx->idx = idx;
   150		return 0;
   151	}
   152	

-- 
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki

Re: [PATCH 2/2] coda: support FSCONFIG_SET_FD for fd mount flag

[kernel test robot]

Hi Aleksa,

kernel test robot noticed the following build errors:

[auto build test ERROR on b80cc4df1124702c600fd43b784e423a30919204]

url:    https://github.com/intel-lab-lkp/linux/commits/Aleksa-Sarai/autofs-fix-missing-fput-for-FSCONFIG_SET_FD/20240718-230056
base:   b80cc4df1124702c600fd43b784e423a30919204
patch link:    https://lore.kernel.org/r/20240719-fsconfig-fsparam_fd-fixes-v1-2-7ccd315c2ad4%40cyphar.com
patch subject: [PATCH 2/2] coda: support FSCONFIG_SET_FD for fd mount flag
config: x86_64-randconfig-161-20240719 (https://download.01.org/0day-ci/archive/20240719/202407190808.l4rcfRij-lkp@intel.com/config)
compiler: clang version 18.1.5 (https://github.com/llvm/llvm-project 617a15a9eac96088ae5e9134248d8236e34b91b1)
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20240719/202407190808.l4rcfRij-lkp@intel.com/reproduce)

If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202407190808.l4rcfRij-lkp@intel.com/

All errors (new ones prefixed by >>):

>> fs/coda/inode.c:129:6: error: use of undeclared identifier 'param'
     129 |         if (param->type == fs_value_is_file) {
         |             ^
   fs/coda/inode.c:130:10: error: use of undeclared identifier 'param'
     130 |                 file = param->file;
         |                        ^
   fs/coda/inode.c:131:3: error: use of undeclared identifier 'param'
     131 |                 param->file = NULL;
         |                 ^
>> fs/coda/inode.c:133:15: error: use of undeclared identifier 'result'
     133 |                 file = fget(result->uint_32);
         |                             ^
   4 errors generated.


vim +/param +129 fs/coda/inode.c

   121	
   122	static int coda_parse_fd(struct fs_context *fc, int fd)
   123	{
   124		struct coda_fs_context *ctx = fc->fs_private;
   125		struct file *file;
   126		struct inode *inode;
   127		int idx;
   128	
 > 129		if (param->type == fs_value_is_file) {
   130			file = param->file;
   131			param->file = NULL;
   132		} else {
 > 133			file = fget(result->uint_32);
   134		}
   135		if (!file)
   136			return -EBADF;
   137	
   138		inode = file_inode(file);
   139		if (!S_ISCHR(inode->i_mode) || imajor(inode) != CODA_PSDEV_MAJOR) {
   140			fput(file);
   141			return invalf(fc, "code: Not coda psdev");
   142		}
   143	
   144		idx = iminor(inode);
   145		fput(file);
   146	
   147		if (idx < 0 || idx >= MAX_CODADEVS)
   148			return invalf(fc, "coda: Bad minor number");
   149		ctx->idx = idx;
   150		return 0;
   151	}
   152	

-- 
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki

Re: [PATCH 2/2] coda: support FSCONFIG_SET_FD for fd mount flag

[Aleksa Sarai]

[-- Attachment #1: Type: text/plain, Size: 3160 bytes --]

On 2024-07-19, kernel test robot <lkp@intel.com> wrote:
> Hi Aleksa,
> 
> kernel test robot noticed the following build errors:
> 
> [auto build test ERROR on b80cc4df1124702c600fd43b784e423a30919204]
> 
> url:    https://github.com/intel-lab-lkp/linux/commits/Aleksa-Sarai/autofs-fix-missing-fput-for-FSCONFIG_SET_FD/20240718-230056
> base:   b80cc4df1124702c600fd43b784e423a30919204
> patch link:    https://lore.kernel.org/r/20240719-fsconfig-fsparam_fd-fixes-v1-2-7ccd315c2ad4%40cyphar.com
> patch subject: [PATCH 2/2] coda: support FSCONFIG_SET_FD for fd mount flag
> config: arm-randconfig-003-20240719 (https://download.01.org/0day-ci/archive/20240719/202407190741.8fA9KJbt-lkp@intel.com/config)
> compiler: arm-linux-gnueabi-gcc (GCC) 14.1.0
> reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20240719/202407190741.8fA9KJbt-lkp@intel.com/reproduce)
> 
> If you fix the issue in a separate patch/commit (i.e. not just a new version of
> the same patch/commit), kindly add following tags
> | Reported-by: kernel test robot <lkp@intel.com>
> | Closes: https://lore.kernel.org/oe-kbuild-all/202407190741.8fA9KJbt-lkp@intel.com/
> 
> All errors (new ones prefixed by >>):
> 
>    fs/coda/inode.c: In function 'coda_parse_fd':
> >> fs/coda/inode.c:129:13: error: 'param' undeclared (first use in this function); did you mean 'parameq'?
>      129 |         if (param->type == fs_value_is_file) {
>          |             ^~~~~
>          |             parameq
>    fs/coda/inode.c:129:13: note: each undeclared identifier is reported only once for each function it appears in
> >> fs/coda/inode.c:133:29: error: 'result' undeclared (first use in this function); did you mean 'mf_result'?
>      133 |                 file = fget(result->uint_32);
>          |                             ^~~~~~
>          |                             mf_result

D'oh. I compile tested it but this fs wasn't enabled in my config...

My bad.

> vim +129 fs/coda/inode.c
> 
>    121	
>    122	static int coda_parse_fd(struct fs_context *fc, int fd)
>    123	{
>    124		struct coda_fs_context *ctx = fc->fs_private;
>    125		struct file *file;
>    126		struct inode *inode;
>    127		int idx;
>    128	
>  > 129		if (param->type == fs_value_is_file) {
>    130			file = param->file;
>    131			param->file = NULL;
>    132		} else {
>  > 133			file = fget(result->uint_32);
>    134		}
>    135		if (!file)
>    136			return -EBADF;
>    137	
>    138		inode = file_inode(file);
>    139		if (!S_ISCHR(inode->i_mode) || imajor(inode) != CODA_PSDEV_MAJOR) {
>    140			fput(file);
>    141			return invalf(fc, "code: Not coda psdev");
>    142		}
>    143	
>    144		idx = iminor(inode);
>    145		fput(file);
>    146	
>    147		if (idx < 0 || idx >= MAX_CODADEVS)
>    148			return invalf(fc, "coda: Bad minor number");
>    149		ctx->idx = idx;
>    150		return 0;
>    151	}
>    152	
> 
> -- 
> 0-DAY CI Kernel Test Service
> https://github.com/intel/lkp-tests/wiki

-- 
Aleksa Sarai
Senior Software Engineer (Containers)
SUSE Linux GmbH
<https://www.cyphar.com/>

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]