2022-08-03 13:46:53

by Chuck Lever III

[permalink] [raw]
Subject: Re: [linux-next:master 13556/14285] ERROR: modpost: "set_posix_acl" [fs/nfsd/nfsd.ko] undefined!



> On Aug 3, 2022, at 2:55 AM, kernel test robot <[email protected]> wrote:
>
> tree: https://git.kernel.org/pub/scm/linux/kernel/git/next/linux-next.git master
> head: 42d670bda02fdba0f3944c92f545984501e5788d
> commit: 2743f3e0444f7287161ecf3e464ee2733dde412d [13556/14285] NFSD: add posix ACLs to struct nfsd_attrs
> config: parisc-defconfig (https://download.01.org/0day-ci/archive/20220803/[email protected]/config)
> compiler: hppa-linux-gcc (GCC) 12.1.0
> reproduce (this is a W=1 build):
> wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
> chmod +x ~/bin/make.cross
> # https://git.kernel.org/pub/scm/linux/kernel/git/next/linux-next.git/commit/?id=2743f3e0444f7287161ecf3e464ee2733dde412d
> git remote add linux-next https://git.kernel.org/pub/scm/linux/kernel/git/next/linux-next.git
> git fetch --no-tags linux-next master
> git checkout 2743f3e0444f7287161ecf3e464ee2733dde412d
> # save the config file
> mkdir build_dir && cp config build_dir/.config
> COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-12.1.0 make.cross W=1 O=build_dir ARCH=parisc SHELL=/bin/bash
>
> If you fix the issue, kindly add following tag where applicable
> Reported-by: kernel test robot <[email protected]>
>
> All errors (new ones prefixed by >>, old ones prefixed by <<):
>
>>> ERROR: modpost: "set_posix_acl" [fs/nfsd/nfsd.ko] undefined!
>
> --
> 0-DAY CI Kernel Test Service
> https://01.org/lkp

Neil, I've reproduced this and confirmed that the following addresses
the error:

464 #ifdef CONFIG_FS_POSIX_ACL
465 if (attr->na_pacl)
466 attr->na_aclerr = set_posix_acl(&init_user_ns,
467 inode, ACL_TYPE_ACCESS,
468 attr->na_pacl);
469 if (!attr->na_aclerr && attr->na_dpacl && S_ISDIR(inode->i_mode))
470 attr->na_aclerr = set_posix_acl(&init_user_ns,
471 inode, ACL_TYPE_DEFAULT,
472 attr->na_dpacl);
473 #endif

I can squash this change into ("NFSD: add posix ACLs to struct nfsd_attrs").

--
Chuck Lever





2022-08-03 23:26:25

by NeilBrown

[permalink] [raw]
Subject: Re: [linux-next:master 13556/14285] ERROR: modpost: "set_posix_acl" [fs/nfsd/nfsd.ko] undefined!

On Wed, 03 Aug 2022, Chuck Lever III wrote:
>
> > On Aug 3, 2022, at 2:55 AM, kernel test robot <[email protected]> wrote:
> >
> > tree: https://git.kernel.org/pub/scm/linux/kernel/git/next/linux-next.git master
> > head: 42d670bda02fdba0f3944c92f545984501e5788d
> > commit: 2743f3e0444f7287161ecf3e464ee2733dde412d [13556/14285] NFSD: add posix ACLs to struct nfsd_attrs
> > config: parisc-defconfig (https://download.01.org/0day-ci/archive/20220803/[email protected]/config)
> > compiler: hppa-linux-gcc (GCC) 12.1.0
> > reproduce (this is a W=1 build):
> > wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
> > chmod +x ~/bin/make.cross
> > # https://git.kernel.org/pub/scm/linux/kernel/git/next/linux-next.git/commit/?id=2743f3e0444f7287161ecf3e464ee2733dde412d
> > git remote add linux-next https://git.kernel.org/pub/scm/linux/kernel/git/next/linux-next.git
> > git fetch --no-tags linux-next master
> > git checkout 2743f3e0444f7287161ecf3e464ee2733dde412d
> > # save the config file
> > mkdir build_dir && cp config build_dir/.config
> > COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-12.1.0 make.cross W=1 O=build_dir ARCH=parisc SHELL=/bin/bash
> >
> > If you fix the issue, kindly add following tag where applicable
> > Reported-by: kernel test robot <[email protected]>
> >
> > All errors (new ones prefixed by >>, old ones prefixed by <<):
> >
> >>> ERROR: modpost: "set_posix_acl" [fs/nfsd/nfsd.ko] undefined!
> >
> > --
> > 0-DAY CI Kernel Test Service
> > https://01.org/lkp
>
> Neil, I've reproduced this and confirmed that the following addresses
> the error:
>
> 464 #ifdef CONFIG_FS_POSIX_ACL
> 465 if (attr->na_pacl)
> 466 attr->na_aclerr = set_posix_acl(&init_user_ns,
> 467 inode, ACL_TYPE_ACCESS,
> 468 attr->na_pacl);
> 469 if (!attr->na_aclerr && attr->na_dpacl && S_ISDIR(inode->i_mode))
> 470 attr->na_aclerr = set_posix_acl(&init_user_ns,
> 471 inode, ACL_TYPE_DEFAULT,
> 472 attr->na_dpacl);
> 473 #endif
>
> I can squash this change into ("NFSD: add posix ACLs to struct nfsd_attrs").

Thanks. I would prefer a fix like

diff --git a/fs/nfsd/vfs.c b/fs/nfsd/vfs.c
index 8a2731d2969d..eeeadd0b2f13 100644
--- a/fs/nfsd/vfs.c
+++ b/fs/nfsd/vfs.c
@@ -455,11 +455,13 @@ nfsd_setattr(struct svc_rqst *rqstp, struct svc_fh *fhp,
if (attr->label && attr->label->len)
attr->label_failed = security_inode_setsecctx(
dentry, attr->label->data, attr->label->len);
- if (attr->pacl)
+ if (IS_ENABLED(CONFIG_FS_POSIX_ACL) &&
+ attr->pacl)
attr->acl_failed = set_posix_acl(&init_user_ns,
inode, ACL_TYPE_ACCESS,
attr->pacl);
- if (!attr->acl_failed && attr->dpacl && S_ISDIR(inode->i_mode))
+ if (IS_ENABLED(CONFIG_FS_POSIX_ACL) &&
+ !attr->acl_failed && attr->dpacl && S_ISDIR(inode->i_mode))
attr->acl_failed = set_posix_acl(&init_user_ns,
inode, ACL_TYPE_DEFAULT,
attr->dpacl);


as that is consistent with similar usage in fs/ksmbd/ and with section
21 "Conditional Compilation' in coding-style.rst

Thanks,
NeilBrown



>
> --
> Chuck Lever
>
>
>
>

2022-08-04 14:37:47

by Chuck Lever III

[permalink] [raw]
Subject: Re: [linux-next:master 13556/14285] ERROR: modpost: "set_posix_acl" [fs/nfsd/nfsd.ko] undefined!

Hi Neil-

> On Aug 3, 2022, at 7:21 PM, NeilBrown <[email protected]> wrote:
>
> On Wed, 03 Aug 2022, Chuck Lever III wrote:
>>
>>> On Aug 3, 2022, at 2:55 AM, kernel test robot <[email protected]> wrote:
>>>
>>> tree: https://git.kernel.org/pub/scm/linux/kernel/git/next/linux-next.git master
>>> head: 42d670bda02fdba0f3944c92f545984501e5788d
>>> commit: 2743f3e0444f7287161ecf3e464ee2733dde412d [13556/14285] NFSD: add posix ACLs to struct nfsd_attrs
>>> config: parisc-defconfig (https://download.01.org/0day-ci/archive/20220803/[email protected]/config)
>>> compiler: hppa-linux-gcc (GCC) 12.1.0
>>> reproduce (this is a W=1 build):
>>> wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
>>> chmod +x ~/bin/make.cross
>>> # https://git.kernel.org/pub/scm/linux/kernel/git/next/linux-next.git/commit/?id=2743f3e0444f7287161ecf3e464ee2733dde412d
>>> git remote add linux-next https://git.kernel.org/pub/scm/linux/kernel/git/next/linux-next.git
>>> git fetch --no-tags linux-next master
>>> git checkout 2743f3e0444f7287161ecf3e464ee2733dde412d
>>> # save the config file
>>> mkdir build_dir && cp config build_dir/.config
>>> COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-12.1.0 make.cross W=1 O=build_dir ARCH=parisc SHELL=/bin/bash
>>>
>>> If you fix the issue, kindly add following tag where applicable
>>> Reported-by: kernel test robot <[email protected]>
>>>
>>> All errors (new ones prefixed by >>, old ones prefixed by <<):
>>>
>>>>> ERROR: modpost: "set_posix_acl" [fs/nfsd/nfsd.ko] undefined!
>>>
>>> --
>>> 0-DAY CI Kernel Test Service
>>> https://01.org/lkp
>>
>> Neil, I've reproduced this and confirmed that the following addresses
>> the error:
>>
>> 464 #ifdef CONFIG_FS_POSIX_ACL
>> 465 if (attr->na_pacl)
>> 466 attr->na_aclerr = set_posix_acl(&init_user_ns,
>> 467 inode, ACL_TYPE_ACCESS,
>> 468 attr->na_pacl);
>> 469 if (!attr->na_aclerr && attr->na_dpacl && S_ISDIR(inode->i_mode))
>> 470 attr->na_aclerr = set_posix_acl(&init_user_ns,
>> 471 inode, ACL_TYPE_DEFAULT,
>> 472 attr->na_dpacl);
>> 473 #endif
>>
>> I can squash this change into ("NFSD: add posix ACLs to struct nfsd_attrs").
>
> Thanks. I would prefer a fix like
>
> diff --git a/fs/nfsd/vfs.c b/fs/nfsd/vfs.c
> index 8a2731d2969d..eeeadd0b2f13 100644
> --- a/fs/nfsd/vfs.c
> +++ b/fs/nfsd/vfs.c
> @@ -455,11 +455,13 @@ nfsd_setattr(struct svc_rqst *rqstp, struct svc_fh *fhp,
> if (attr->label && attr->label->len)
> attr->label_failed = security_inode_setsecctx(
> dentry, attr->label->data, attr->label->len);
> - if (attr->pacl)
> + if (IS_ENABLED(CONFIG_FS_POSIX_ACL) &&
> + attr->pacl)
> attr->acl_failed = set_posix_acl(&init_user_ns,
> inode, ACL_TYPE_ACCESS,
> attr->pacl);
> - if (!attr->acl_failed && attr->dpacl && S_ISDIR(inode->i_mode))
> + if (IS_ENABLED(CONFIG_FS_POSIX_ACL) &&
> + !attr->acl_failed && attr->dpacl && S_ISDIR(inode->i_mode))
> attr->acl_failed = set_posix_acl(&init_user_ns,
> inode, ACL_TYPE_DEFAULT,
> attr->dpacl);
>
>
> as that is consistent with similar usage in fs/ksmbd/ and with section
> 21 "Conditional Compilation' in coding-style.rst

I didn't realize:

> However, this approach still allows the C compiler to see the code
> inside the block, and check it for correctness (syntax, types, symbol
> references, etc).


I've applied your suggestion, compile-tested, and pushed it to the
NFSD for-next branch on kernel.org.


--
Chuck Lever