Message ID | 20230331123221.3273328-2-roberto.sassu@huaweicloud.com (mailing list archive) |
---|---|
State | Accepted |
Delegated to: | Paul Moore |
Headers | show |
Series | evm: Do HMAC of multiple per LSM xattrs for new inodes | expand |
On Fri, Mar 31, 2023 at 8:33 AM Roberto Sassu <roberto.sassu@huaweicloud.com> wrote: > > From: Roberto Sassu <roberto.sassu@huawei.com> > > Reiserfs sets a security xattr at inode creation time in two stages: first, > it calls reiserfs_security_init() to obtain the xattr from active LSMs; > then, it calls reiserfs_security_write() to actually write that xattr. > > Unfortunately, it seems there is a wrong expectation that LSMs provide the > full xattr name in the form 'security.<suffix>'. However, LSMs always > provided just the suffix, causing reiserfs to not write the xattr at all > (if the suffix is shorter than the prefix), or to write an xattr with the > wrong name. > > Add a temporary buffer in reiserfs_security_write(), and write to it the > full xattr name, before passing it to reiserfs_xattr_set_handle(). > > Also replace the name length check with a check that the full xattr name is > not larger than XATTR_NAME_MAX. > > Cc: stable@vger.kernel.org # v2.6.x > Fixes: 57fe60df6241 ("reiserfs: add atomic addition of selinux attributes during inode creation") > Signed-off-by: Roberto Sassu <roberto.sassu@huawei.com> > --- > fs/reiserfs/xattr_security.c | 8 ++++++-- > 1 file changed, 6 insertions(+), 2 deletions(-) This looks good to me, thanks. While normally I would merge something like this into the lsm/stable-X.Y branch, I'm going to merge it into lsm/next to give it a week or two of extra testing. I think anyone who is using reiserfs+LSM (doubtful as it looks horribly broken) would be okay with waiting a few more days at this point :)
diff --git a/fs/reiserfs/xattr_security.c b/fs/reiserfs/xattr_security.c index 6bffdf9a4fd..6e0a099dd78 100644 --- a/fs/reiserfs/xattr_security.c +++ b/fs/reiserfs/xattr_security.c @@ -95,11 +95,15 @@ int reiserfs_security_write(struct reiserfs_transaction_handle *th, struct inode *inode, struct reiserfs_security_handle *sec) { + char xattr_name[XATTR_NAME_MAX + 1] = XATTR_SECURITY_PREFIX; int error; - if (strlen(sec->name) < sizeof(XATTR_SECURITY_PREFIX)) + + if (XATTR_SECURITY_PREFIX_LEN + strlen(sec->name) > XATTR_NAME_MAX) return -EINVAL; - error = reiserfs_xattr_set_handle(th, inode, sec->name, sec->value, + strlcat(xattr_name, sec->name, sizeof(xattr_name)); + + error = reiserfs_xattr_set_handle(th, inode, xattr_name, sec->value, sec->length, XATTR_CREATE); if (error == -ENODATA || error == -EOPNOTSUPP) error = 0;