Stable backport of de3004c874e7 ("ocfs2: Switch to security_inode_init_security()")

Stable backport of de3004c874e7 ("ocfs2: Switch to security_inode_init_security()")

[Paul Moore]

Hello,

I would like to request the backport of the commit below to address a
kernel panic in ocfs2 that was identified by Valentin Vidić in this
thread:

https://lore.kernel.org/linux-security-module/20230401214151.1243189-1-vvidic@valentin-vidic.from.hr

While Valentin provides his own patch in the original message, the
preferred patch is one that went up to Linus during the last merge
window; Valentin has tested the patch and confirmed that it resolved
the reported problem.

  commit de3004c874e740304cc4f4a83d6200acb511bbda
  Author: Roberto Sassu <roberto.sassu@huawei.com>
  Date:   Tue Mar 14 09:17:16 2023 +0100

   ocfs2: Switch to security_inode_init_security()

   In preparation for removing security_old_inode_init_security(), switch to
   security_inode_init_security().

   Extend the existing ocfs2_initxattrs() to take the
   ocfs2_security_xattr_info structure from fs_info, and populate the
   name/value/len triple with the first xattr provided by LSMs.

   As fs_info was not used before, ocfs2_initxattrs() can now handle the case
   of replicating the behavior of security_old_inode_init_security(), i.e.
   just obtaining the xattr, in addition to setting all xattrs provided by
   LSMs.

   Supporting multiple xattrs is not currently supported where
   security_old_inode_init_security() was called (mknod, symlink), as it
   requires non-trivial changes that can be done at a later time. Like for
   reiserfs, even if EVM is invoked, it will not provide an xattr (if it is
   not the first to set it, its xattr will be discarded; if it is the first,
   it does not have xattrs to calculate the HMAC on).

   Finally, since security_inode_init_security(), unlike
   security_old_inode_init_security(), returns zero instead of -EOPNOTSUPP if
   no xattrs were provided by LSMs or if inodes are private, additionally
   check in ocfs2_init_security_get() if the xattr name is set.

   If not, act as if security_old_inode_init_security() returned -EOPNOTSUPP,
   and set si->enable to zero to notify to the functions following
   ocfs2_init_security_get() that no xattrs are available.

   Signed-off-by: Roberto Sassu <roberto.sassu@huawei.com>
   Reviewed-by: Casey Schaufler <casey@schaufler-ca.com>
   Acked-by: Joseph Qi <joseph.qi@linux.alibaba.com>
   Reviewed-by: Mimi Zohar <zohar@linux.ibm.com>
   Signed-off-by: Paul Moore <paul@paul-moore.com>

-- 
paul-moore.com

Re: Stable backport of de3004c874e7 ("ocfs2: Switch to security_inode_init_security()")

[Tetsuo Handa]

On 2023/05/20 6:51, Paul Moore wrote:
>    Finally, since security_inode_init_security(), unlike
>    security_old_inode_init_security(), returns zero instead of -EOPNOTSUPP if
>    no xattrs were provided by LSMs or if inodes are private, additionally
>    check in ocfs2_init_security_get() if the xattr name is set.
> 
>    If not, act as if security_old_inode_init_security() returned -EOPNOTSUPP,
>    and set si->enable to zero to notify to the functions following
>    ocfs2_init_security_get() that no xattrs are available.

Regarding security_inode_init_security(), similar problem was found on reiserfs.

https://lkml.kernel.org/r/a800496b-cae9-81bf-c79e-d8342418c5be@I-love.SAKURA.ne.jp

Is it really expected behavior that security_inode_init_security() returns 0 when
initxattrs is provided but call_int_hook(inode_init_security) returned -EOPNOTSUPP ?

Re: Stable backport of de3004c874e7 ("ocfs2: Switch to security_inode_init_security()")

[Paul Moore]

On Sat, May 20, 2023 at 1:54 AM Tetsuo Handa
<penguin-kernel@i-love.sakura.ne.jp> wrote:
> On 2023/05/20 6:51, Paul Moore wrote:
> >    Finally, since security_inode_init_security(), unlike
> >    security_old_inode_init_security(), returns zero instead of -EOPNOTSUPP if
> >    no xattrs were provided by LSMs or if inodes are private, additionally
> >    check in ocfs2_init_security_get() if the xattr name is set.
> >
> >    If not, act as if security_old_inode_init_security() returned -EOPNOTSUPP,
> >    and set si->enable to zero to notify to the functions following
> >    ocfs2_init_security_get() that no xattrs are available.
>
> Regarding security_inode_init_security(), similar problem was found on reiserfs ...

It is likely that reiserfs lhas other issues as well, which is why I
didn't propose the similar reiserfs patch for backporting to stable.
The request here is only for the ocfs2 patch, which has not seen any
problem reports and does fix at least one reported bug/panic.

Let's discuss the reiserfs patches in the other, relevant threads and
leave this thread dedicated to the ocfs2 backport to stable.

-- 
paul-moore.com

Re: Stable backport of de3004c874e7 ("ocfs2: Switch to security_inode_init_security()")

[Roberto Sassu]

On Sat, 2023-05-20 at 14:54 +0900, Tetsuo Handa wrote:
> On 2023/05/20 6:51, Paul Moore wrote:
> >    Finally, since security_inode_init_security(), unlike
> >    security_old_inode_init_security(), returns zero instead of -EOPNOTSUPP if
> >    no xattrs were provided by LSMs or if inodes are private, additionally
> >    check in ocfs2_init_security_get() if the xattr name is set.
> > 
> >    If not, act as if security_old_inode_init_security() returned -EOPNOTSUPP,
> >    and set si->enable to zero to notify to the functions following
> >    ocfs2_init_security_get() that no xattrs are available.
> 
> Regarding security_inode_init_security(), similar problem was found on reiserfs.
> 
> https://lkml.kernel.org/r/a800496b-cae9-81bf-c79e-d8342418c5be@I-love.SAKURA.ne.jp
> 
> Is it really expected behavior that security_inode_init_security() returns 0 when
> initxattrs is provided but call_int_hook(inode_init_security) returned -EOPNOTSUPP ?

It is going to change with this upcoming patch:

https://lore.kernel.org/linux-integrity/20230331123221.3273328-3-roberto.sassu@huaweicloud.com/

There will be only one loop, and error handling would be the same.

Thanks

Roberto

Re: Stable backport of de3004c874e7 ("ocfs2: Switch to security_inode_init_security()")

[Sasha Levin]

On Fri, May 19, 2023 at 05:51:59PM -0400, Paul Moore wrote:
>Hello,
>
>I would like to request the backport of the commit below to address a
>kernel panic in ocfs2 that was identified by Valentin Vidić in this
>thread:
>
>https://lore.kernel.org/linux-security-module/20230401214151.1243189-1-vvidic@valentin-vidic.from.hr
>
>While Valentin provides his own patch in the original message, the
>preferred patch is one that went up to Linus during the last merge
>window; Valentin has tested the patch and confirmed that it resolved
>the reported problem.

How far should this patch be backported?

-- 
Thanks,
Sasha

Re: Stable backport of de3004c874e7 ("ocfs2: Switch to security_inode_init_security()")

[Paul Moore]

On Mon, May 22, 2023 at 12:07 PM Sasha Levin <sashal@kernel.org> wrote:
> On Fri, May 19, 2023 at 05:51:59PM -0400, Paul Moore wrote:
> >Hello,
> >
> >I would like to request the backport of the commit below to address a
> >kernel panic in ocfs2 that was identified by Valentin Vidić in this
> >thread:
> >
> >https://lore.kernel.org/linux-security-module/20230401214151.1243189-1-vvidic@valentin-vidic.from.hr
> >
> >While Valentin provides his own patch in the original message, the
> >preferred patch is one that went up to Linus during the last merge
> >window; Valentin has tested the patch and confirmed that it resolved
> >the reported problem.
>
> How far should this patch be backported?

The problem is only present when the BPF LSM is enabled, which I
believe was merged upstream in the v5.7 release, so anything v5.7 or
later should be affected and thus a good backport target.

-- 
paul-moore.com

Re: Stable backport of de3004c874e7 ("ocfs2: Switch to security_inode_init_security()")

[Greg KH]

On Mon, May 22, 2023 at 12:34:11PM -0400, Paul Moore wrote:
> On Mon, May 22, 2023 at 12:07 PM Sasha Levin <sashal@kernel.org> wrote:
> > On Fri, May 19, 2023 at 05:51:59PM -0400, Paul Moore wrote:
> > >Hello,
> > >
> > >I would like to request the backport of the commit below to address a
> > >kernel panic in ocfs2 that was identified by Valentin Vidić in this
> > >thread:
> > >
> > >https://lore.kernel.org/linux-security-module/20230401214151.1243189-1-vvidic@valentin-vidic.from.hr
> > >
> > >While Valentin provides his own patch in the original message, the
> > >preferred patch is one that went up to Linus during the last merge
> > >window; Valentin has tested the patch and confirmed that it resolved
> > >the reported problem.
> >
> > How far should this patch be backported?
> 
> The problem is only present when the BPF LSM is enabled, which I
> believe was merged upstream in the v5.7 release, so anything v5.7 or
> later should be affected and thus a good backport target.

Thanks, now queued up.

greg k-h