| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <CAEjxPJ4uoqkwmbeOOmUHJwoKiK3cnfJP5P2w+hOXr4BZNtQ+JQ@mail.gmail.com>
Date: Mon, 11 Sep 2023 09:49:27 -0400
From: Stephen Smalley <stephen.smalley.work@...il.com>
To: kernel test robot <lkp@...el.com>
Cc: Alfred Piccioni <alpic@...gle.com>,
Paul Moore <paul@...l-moore.com>,
Eric Paris <eparis@...isplace.org>,
oe-kbuild-all@...ts.linux.dev, stable@...r.kernel.org,
selinux@...r.kernel.org, linux-kernel@...r.kernel.org,
LSM List <linux-security-module@...r.kernel.org>
Subject: Re: [PATCH V2] SELinux: Check correct permissions for FS_IOC32_*
On Mon, Sep 11, 2023 at 9:19 AM Stephen Smalley
<stephen.smalley.work@...il.com> wrote:
>
> On Fri, Sep 8, 2023 at 6:54 PM kernel test robot <lkp@...el.com> wrote:
> >
> > Hi Alfred,
> >
> > kernel test robot noticed the following build errors:
> >
> > [auto build test ERROR on 50a510a78287c15cee644f345ef8bac8977986a7]
> >
> > url: https://github.com/intel-lab-lkp/linux/commits/Alfred-Piccioni/SELinux-Check-correct-permissions-for-FS_IOC32_/20230906-200131
> > base: 50a510a78287c15cee644f345ef8bac8977986a7
> > patch link: https://lore.kernel.org/r/20230906115928.3749928-1-alpic%40google.com
> > patch subject: [PATCH V2] SELinux: Check correct permissions for FS_IOC32_*
> > config: i386-debian-10.3 (https://download.01.org/0day-ci/archive/20230909/202309090600.NSyo7d2q-lkp@intel.com/config)
> > compiler: gcc-12 (Debian 12.2.0-14) 12.2.0
> > reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20230909/202309090600.NSyo7d2q-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@...el.com>
> > | Closes: https://lore.kernel.org/oe-kbuild-all/202309090600.NSyo7d2q-lkp@intel.com/
> >
> > All errors (new ones prefixed by >>):
> >
> > security/selinux/hooks.c: In function 'selinux_file_ioctl':
> > >> security/selinux/hooks.c:3647:9: error: duplicate case value
> > 3647 | case FS_IOC32_GETFLAGS:
> > | ^~~~
> > security/selinux/hooks.c:3645:9: note: previously used here
> > 3645 | case FS_IOC_GETFLAGS:
> > | ^~~~
> > security/selinux/hooks.c:3648:9: error: duplicate case value
> > 3648 | case FS_IOC32_GETVERSION:
> > | ^~~~
> > security/selinux/hooks.c:3646:9: note: previously used here
> > 3646 | case FS_IOC_GETVERSION:
> > | ^~~~
> > security/selinux/hooks.c:3654:9: error: duplicate case value
> > 3654 | case FS_IOC32_SETFLAGS:
> > | ^~~~
> > security/selinux/hooks.c:3652:9: note: previously used here
> > 3652 | case FS_IOC_SETFLAGS:
> > | ^~~~
> > security/selinux/hooks.c:3655:9: error: duplicate case value
> > 3655 | case FS_IOC32_SETVERSION:
> > | ^~~~
> > security/selinux/hooks.c:3653:9: note: previously used here
> > 3653 | case FS_IOC_SETVERSION:
> > | ^~~~
>
> Not sure of the right way to fix this while addressing the original
> issue that this patch was intended to fix. Looking in fs/ioctl.c, I
> see that the some FS_IOC32 values are remapped to the corresponding
> FS_IOC values by the compat ioctl syscall entrypoint. Also notice this
> comment there:
>
> /* RED-PEN how should LSM module know it's handling 32bit? */
> error = security_file_ioctl(f.file, cmd, arg);
> if (error)
> goto out;
>
> So perhaps this is a defect in LSM that needs to be addressed?
Note btw that some of the 32-bit ioctl commands are only handled in
the fs-specific compat_ioctl routines, e.g. ext4_compat_ioctl()
handles EXT4_IOC32_GETVERSION == FS_IOC32_GETVERSION and ditto for
_SETVERSION.
>
>
> >
> >
> > vim +3647 security/selinux/hooks.c
> >
> > 3634
> > 3635 static int selinux_file_ioctl(struct file *file, unsigned int cmd,
> > 3636 unsigned long arg)
> > 3637 {
> > 3638 const struct cred *cred = current_cred();
> > 3639 int error = 0;
> > 3640
> > 3641 switch (cmd) {
> > 3642 case FIONREAD:
> > 3643 case FIBMAP:
> > 3644 case FIGETBSZ:
> > 3645 case FS_IOC_GETFLAGS:
> > 3646 case FS_IOC_GETVERSION:
> > > 3647 case FS_IOC32_GETFLAGS:
> > 3648 case FS_IOC32_GETVERSION:
> > 3649 error = file_has_perm(cred, file, FILE__GETATTR);
> > 3650 break;
> > 3651
> > 3652 case FS_IOC_SETFLAGS:
> > 3653 case FS_IOC_SETVERSION:
> > 3654 case FS_IOC32_SETFLAGS:
> > 3655 case FS_IOC32_SETVERSION:
> > 3656 error = file_has_perm(cred, file, FILE__SETATTR);
> > 3657 break;
> > 3658
> > 3659 /* sys_ioctl() checks */
> > 3660 case FIONBIO:
> > 3661 case FIOASYNC:
> > 3662 error = file_has_perm(cred, file, 0);
> > 3663 break;
> > 3664
> > 3665 case KDSKBENT:
> > 3666 case KDSKBSENT:
> > 3667 error = cred_has_capability(cred, CAP_SYS_TTY_CONFIG,
> > 3668 CAP_OPT_NONE, true);
> > 3669 break;
> > 3670
> > 3671 case FIOCLEX:
> > 3672 case FIONCLEX:
> > 3673 if (!selinux_policycap_ioctl_skip_cloexec())
> > 3674 error = ioctl_has_perm(cred, file, FILE__IOCTL, (u16) cmd);
> > 3675 break;
> > 3676
> > 3677 /* default case assumes that the command will go
> > 3678 * to the file's ioctl() function.
> > 3679 */
> > 3680 default:
> > 3681 error = ioctl_has_perm(cred, file, FILE__IOCTL, (u16) cmd);
> > 3682 }
> > 3683 return error;
> > 3684 }
> > 3685
Powered by blists - more mailing lists